Proposal: Unnested Local Functions #1329
Replies: 40 comments
-
you won't be able to capture any variable in that context, therefore I guess it wouldn't be much different from a regular method. |
Beta Was this translation helpful? Give feedback.
-
I think it can conflict with explicit interface implements. on the other hand, you can write a regular method. you get pretty much same thing. |
Beta Was this translation helpful? Give feedback.
-
@alrz @MkazemAkhgary
This is not the place to discuss using local functions or not. C# added local functions already, and I suggest an alternative syntax for them. In the code I linked, I put the local function inside #region and but comments and did every thing to make the code looks clear in the editor, but I still feel something is not in place. At least I can't find this function easily, because it doesn't appear in method names dropdown list. |
Beta Was this translation helpful? Give feedback.
-
Where inside of
How do you write an unnested "local" function for a method with more than one overload? Other potentially problematic cases: an unnested "local" function for a property accessor, a constructor or an explicit interface implementation.
If you want the IDE to behave differently, you should propose that the IDE should behave differently, not that the language should change. Especially because changing the IDE is much easier than changing the language. |
Beta Was this translation helpful? Give feedback.
-
The whole point of a local function is that it's local. What this is suggesting is just a private method with new and confusing syntax. If you want to define it in another scope there's already plenty of syntax for that class A
{
void Foo1()
{
Foo1Locals.Foo2();
}
static class Foo1Locals
{
internal static void Foo2() {}
}
} |
Beta Was this translation helpful? Give feedback.
-
Totally agree. @MohammadHamdyGhanem Note that these are new language features and it takes some time to for IDE to amend and cope with these new features. |
Beta Was this translation helpful? Give feedback.
-
I could see some benefit in being able to define a local function in a partial class. partial class Foo
{
public void DoSomething()
{
// ...
var items = EnumerateItems();
// ...
IEnumerable<Item> EnumerateItems();
}
} partial class Foo
{
local IEnumerable<Item> DoSomething().EnumerateItems()
{
// implementation
}
} Here I explicitly specified the placement of the local method, but perhaps there could be a default location as mentioned in comments above. I also disambiguated the method signature, in case of overloads. |
Beta Was this translation helpful? Give feedback.
-
|
Beta Was this translation helpful? Give feedback.
-
@svick
|
Beta Was this translation helpful? Give feedback.
-
@MohammadHamdyGhanem You misunderstood. I'm not asking how to define overloads of a local function (which is not allowed for local functions in the first place). What I'm asking is: void foo1() {}
void foo1(int i)
{
// how do I change this local function to unnested?
void foo2() {}
} You can probably invent some syntax to reference overloads, but it won't be trivial and the language designers seem to be reluctant to do it (see #712 and this article). |
Beta Was this translation helpful? Give feedback.
-
@svick
This can be re-written as:
|
Beta Was this translation helpful? Give feedback.
-
@bondsbw |
Beta Was this translation helpful? Give feedback.
-
I don't think anything outside of a method's scope should be able to access the methods locals, arguments, or local functions. Too confusing for zero benefit. If it's complex enough to deserve strict organization, it's too complex to use local functions for and you should move it to its own class. |
Beta Was this translation helpful? Give feedback.
-
nothing is out the scope. Please note this is just away to organize the code in the editor, and doesn't affect how IL is generated. All what C# will do is to replace the local function definition line with its imlemetation body! Something like partial methods. |
Beta Was this translation helpful? Give feedback.
-
It is lexically out of scope. The method scope ends with |
Beta Was this translation helpful? Give feedback.
-
I don't understand how this proposal addresses that. You're still using local functions, you're just putting their body somewhere else. |
Beta Was this translation helpful? Give feedback.
-
ok... so don't use them? |
Beta Was this translation helpful? Give feedback.
-
Had to then. I want to make my existing code more organized. |
Beta Was this translation helpful? Give feedback.
-
Again, i don't see how moving the local function delcaration to be a sibling addresses your problems 'with complex tightly coubled un-reuseable code!' The code is just as tightly coupled as before. The code is just as unreusable as before. All that has changed is that you've moved the location of the code to a slightly different location. |
Beta Was this translation helpful? Give feedback.
-
I don't understand this comment. I can't imagine a single case where someone would "have to" use local functions. Indeed, we had the language for 15ish years without local functions, and life was just fine. You would only use local functions by choice, not because of any compulsion :) |
Beta Was this translation helpful? Give feedback.
-
It comes down to a combination of organizational preference and problem domain. As much as I like having local functions in the language, I don't use them as much as I expected to. C# is a very expressive language and, for what really is a very minimal amount of syntactic pain, it has allowed us to leverage lexical scope to our hearts content for well over a decade 😀 IEnumerable<(Action<T>, int)> ThingsWeMightDoInTheFuture<T>(IEnumerable<T> elements) =>
from outer in elements
join inner in elements
on outer.Key equals inner.Key into correlated
let merge = (Func<T, int>)(x => correlated.Count(e => e.Equals(x)))
select ((Action<T>)(e => e.Rank = merge(e)), correlated.Aggregate(1, (a, e) => a * e.Key)); In other words, local functions have not changed how I code all that much and you certainly don't need to use them to code yourself up a mess of lexically nested scopes. |
Beta Was this translation helpful? Give feedback.
-
If foo2() works on many variables of foo1() and makes changes to them, then using a local function is faster than declaring a regular function with too many ref params. Did you use a function with 20 params before? |
Beta Was this translation helpful? Give feedback.
-
Alright, then i don't know what your position is :) You made some claims, but haven't explained them. So i'm really unsure what the problem is, or why this proposal is a suitable approach to address whatever that problem actually is :-/ |
Beta Was this translation helpful? Give feedback.
-
The proposal is what is written in the first post. No more.
|
Beta Was this translation helpful? Give feedback.
-
This is not a good justification/example. The purpose of partial methods is to allow scenarios where one producer of an API defines an extension point that someone else can fill in. They can be not filled in, and then have no impact on what's compiled. There is no such reason/benefit to your proposal. I'm not sure what the benefit of this is or what problem it is addressing. It just seems to make things more confusing. it moves the code away from where you need it. If you want to write things this way then just write things like this: public test()
{
var foo5 = test_foo5();
}
Func<int> test_foo5()
{
return () => ...
} Finally, these examples are completely non-compelling. Your problem example doens't seem like a problme, and your solution doesn't demonstrate why this would be desirable. Please provide realistic examples to help drive home the case. For example, see this PR as one that shows a much more realistic example that demonstrates why a solution is needed: #1328 |
Beta Was this translation helpful? Give feedback.
-
You have a function, So now, you are thinking, "well if I could move those local functions outside of All along, you are ignoring the elephant in the room: those 20 variables. They have been telling you from the start that you are trying to do too much in one function. You need to reduce them. There is a range of solutions you could employ. At one end of that spectrum lies the "ideal" solution: rewrite the entire function to simplify it. Of course to confidently do this, you'll need a solid set of unit tests backing you up. Apologies if I'm misjudging you, but I'm going to guess that you don't have unit tests. At the other end of the spectrum is "fudge it". Take those 20 variables, and make them fields in your class. Normally, I'd rather chew off my own right hand than suggest someone do this, but I think it likely the best solution in your case. You'll still be using "action at a distance", but it'll let you chop Local functions are a useful addition to the language. Please do not confuse your attempted misuse of them with signs that they are a bad idea. Thanks. |
Beta Was this translation helpful? Give feedback.
-
@DavidArno |
Beta Was this translation helpful? Give feedback.
-
If I look at, eg But if you find yourself thrashing around trying to shoehorn your code into inappropriate features because it's long, complicated and unreadable, then those many variables clearly aren't fine. There's no hard and fast rule on this, but if you have a local function that's more then say half dozen lines long, then alarm bells should ring that maybe a local function isn't the right solution. |
Beta Was this translation helpful? Give feedback.
-
This is actually exactly what the compiler already does. public static void Main()
{
int Foo() => 3;
}
// Becomes:
public static class Program
{
public static void Main()
{
}
[CompilerGenerated]
internal static int <Main>g__Foo|0_0()
{
return 3;
}
} Unlike nested functions in JavaScript, Local Functions aren't actually contained in the outer function, they get moved outside. This makes them more efficient. So, what I don't get, is what are we gaining in doing it in your syntax? Local functions are already organized, inside the function they belong in. We shouldn't have to see the function outside of it because it will make things more complicated. If you're worried about your outer method being really huge now, that's fine. Jon Skeet said his methods with local functions get big and it looks like it needs to be refactored, except it already is, and everything is nicely scoped. You could do: public static void Main()
{
#region Local Functions
int Foo() => 3;
#endregion Local Functions
} |
Beta Was this translation helpful? Give feedback.
-
To my mind, putting a |
Beta Was this translation helpful? Give feedback.
-
I used local functions in practice (as the CopyVideo() function inside the btnSave_Click here:
https://github.com/MohammadHamdyGhanem/VASE/blob/master/src/Forms/VideoEditSaver.cs
I found it the best solution in my situation, but it is still confusing. It is not a good organization to put a full a function among other lines of code. So, I suggest to allow us to re-write this:
using "local" keyword as this:
Note:
foo2 still can not be called from any method other than foo1. This is just away to organize the code in the editor, and doesn't affect how IL is generated. All what C# will do is to replace the local function definition line with its imlemetation body! Something like partial methods.
The name foo1.foo2 should appear in method names dropdown list (in the top of the code editor page).
Properties can be treated in the same way:
Constructors can be refered to as the name of the class like this:
This can be re-written as:
I think it will make the code more organized and easier to read, with the same functionality.
Beta Was this translation helpful? Give feedback.
All reactions