Local Functions - Yes or no?

I’m thinking of heavily using these from now on. From what I can tell, they seem amazing.
They enable me to understand what a class is actually doing much faster. If I see a local method with a good name, I couldn’t care less about the implementation, I simply know it’s used in one single location and nowhere else in the entire project. I don’t care about it being private/protected/public etc. I can just collapse it and not look at it, I know it does something to some part of the code that I don’t care at this point.

However, I want your opinion here, do they reduce readability?
Example 1

class Example
{
    private void MethodA()
    {
        MethodB();            // Calling
   
        void MethodB()
        {
            // MethodB implementation
        }
    }
}

If you asked me, in this example, they don’t. They are super clear and easy to understand.

But, what if we start nesting them?
Example 2

class Example
{
    private void MethodA()
    {
        MethodB();            // Calling
   
        void MethodB()
        {
            MethodC();        // Calling
           
            void MethodC()
            {
                // MethodC implementation
            }
        }
    }
}

Here I believe code readability can suffer. I still look at this as an improvement, since I know exactly what depends on what and what is used where, but it’s ugly as hell.

What’s your input on this? Do you recommend only going X levels deep or just completely avoiding these? I don’t really see a point in limiting yourself to the depth of these, since you are breaking any consistency in your code that you built up to that point.

I’d love to hear some feedback on these :slight_smile:

I think local functions were a mistake that should have never been added to the language. “Collapsing” them for readability is only possible if your IDE supports it, and there is no way to persist that state across developers etc… So the default state is that you see it. I don’t see what benefit it has over a local lambda, which itself is only rarely useful, at least in my personal code style.

The bigger issue with it though is that, for beginner programmers, it’s a huge pitfall. I see so many posts on this forum where people new to Unity have put their Unity callback functions inside some other function. For example, OnCollisionEnter as a local function inside Update. Obviously this isn’t going to work, but since local functions are now valid C#, it fails silently. What would have been a quick compile error is now a silent confusing error for a new user who is already struggling to understand the concepts of variable scoping.

3 Likes

The only times I use local functions are in regards to writing custom editor functionality in MonoBehavior scripts, so that editor-only functions don’t show up in Intellisense, mitigating the risk of potentially calling an editor-only function in the standalone build accidentally.

For example:

public class MyMonoBehaviour : MonoBehavior {
   /*
    * Regular MonoBehavior stuff up here...
    */

#if UNITY_EDITOR
   void OnDrawGizmosSelected() {
      GetTheStuff();
      DrawTheShapes();
      LogTheThings();

      void GetTheStuff() {
         //etc...
      }

      void DrawTheShapes() {
         //etc...
      }

      void LogTheThings() {
         //etc...
      }
   }
#endif
}
2 Likes

I use local functions exclusively not to repeat myself in monolithic functions that do something algorithmically, as well as in deep recursions that are local to the method body. It is even recommended to do so.

For example, large text parsers can have complex processors that are reused in one go. But I have other examples as well.

Lambdas are anonymous and require the compiler to create a delegate (which is a class behind the scenes), to which they’re assigned. All of this unnecessarily bloats the generated binary, and probably affects the performance as well. But most importantly, the code is less readable in some cases.

Also, unlike lambdas, local methods have a decent opportunity to be locally inlined by a compiler. Now, from what I can gather, this isn’t guaranteed depending on many factors, but still makes them more attractive imo.

However, if the code is more readable by using lambda, I’d go with a lambda instead. Though tbh, I try to avoid lambdas in core design and use them only in the very high level. You know a ctor somewhere wants to sort a short list once, that kind of thing – ad hoc comparators and such. I definitely don’t write delegates or local methods for that.

But if I’m making a custom collection or a custom service, I typically declare my own delegates, or if it makes sense (and it frequently does) I reuse the generic ones: System.Func<> or System.Action<>.

2 Likes

They’re very, very good.

You can improve readability of code by a lot by giving parts names, instead of:

void DoLargeThing() {
    code;
    code;
    ...
    code;
}

You can break it down to:

void DoLargeThing() {
    DoPart1();
    DoPart2();
    DoPart3();
}

The names of those smaller things both makes the code easier to skim, and makes what you’re trying to do with the implementation clearer to the reader.

The problem is that when those sub-functions are outside the body of the function they’re used in, they end up looking like they are more general purpose than they really are. You can also end up with a lot of clutter where it’s hard to know which functions belongs where.

Enclosing all of the sub-methods in the method they’re used in works around that, and makes it really clear to the reader that that’s where they belong.

I do believe that you should avoid having nested methods in the middle of the method they’re nested in. So, in general, try to do this:

void DoLargeThing() {
    DoPart1();
    DoPart2();
    DoPart3();

    void DoPart1() {
        ...
    }

    void DoPart2() {
        ...
    }

    void DoPart3() {
        ...
    }
}

Do not do this:

void DoLargeThing() {
    DoPart1();

    void DoPart1() {
        ...
    }

    DoPart2();

    void DoPart2() {
        ...
    }

    DoPart3();
    void DoPart3() {
        ...
    }
}

because that gets very confusing!

I also try to avoid nesting them too deep, as that leads to so much indentation that you barely have room for the code.

They perform better than lambdas, and have names, so that’s clear advantages. The lambda’s also have to be declared where they’re used, instead of wherever, so the comparison is kinda off?

A language should for sure try to be somewhat easy to read, and not force new users to learn too much to be able to use them. But the errors you’re talking about here is that beginner programmers doesn’t understand scope, and that’s so fundamental that a language should be able to assume that people will understand what that is before they use it.

6 Likes