-1

Found the following code in one open source project, and I am amazed how original coder used local and static local functions in here:

public static Win32Error CallMethodWithTypedBuf<TOut, TSize>(SizeFunc<TSize> getSize, PtrFunc<TSize> method, out TOut result, Func<IntPtr, TSize, TOut> outConverter = null, Win32Error? bufErr = null) where TSize : struct, IConvertible
{
    TSize sz = default;
    result = default;
    var err = (getSize ?? GetSize)(ref sz);
    if (err.Failed && (bufErr == null || bufErr.Value != err) && !buffErrs.Contains(err)) return err;
    using (var buf = new SafeHGlobalHandle(sz.ToInt32(null)))
    {
        err = method(buf.DangerousGetHandle(), ref sz);
        if (err.Succeeded)
            result = (outConverter ?? Conv)(buf.DangerousGetHandle(), sz);
        return err;
    }

    Win32Error GetSize(ref TSize sz1) => method(IntPtr.Zero, ref sz1);

    static TOut Conv(IntPtr p, TSize s) => p == IntPtr.Zero ? default : p.Convert<TOut>(Convert.ToUInt32(s));
}

So I've rewritten to something that would actually have logic in one place, and would work even in older compilers (As it would be written 10 years ago). To realize that following code is actually less in size, consumes less call stack, and could be better optimized by if-branch prediction:

public static Win32Error CallMethodWithTypedBuf<TOut, TSize>(SizeFunc<TSize> getSize, PtrFunc<TSize> method, out TOut result, Func<IntPtr, TSize, TOut> outConverter = null, Win32Error? bufErr = null) where TSize : struct, IConvertible
{
    TSize sz = default;
    result = default;
    var err = (null != getSize ? getSize(ref sz) : method(IntPtr.Zero, ref sz));
    if (err.Failed && (bufErr == null || bufErr.Value != err) && !buffErrs.Contains(err)) return err;
    using (var buf = new SafeHGlobalHandle(sz.ToInt32(null)))
    {
        err = method(buf.DangerousGetHandle(), ref sz);
if (err.Succeeded)
{
  IntPtr p = buf.DangerousGetHandle();
  result = (null != outConverter) ? outConverter(p, sz) : (p == IntPtr.Zero ? default : p.Convert<TOut>(Convert.ToUInt32(sz)));
}
        return err;
    }           
}

So I'am more than little confused why would anyone use local and static functions, as original coder did use. At this point I think people are angry and do not know what to do with themselfs, and started making mess at global scale. Please tell me that I am wrong, and that we all should use local and static local functions immediately all over our codes?

SoLaR
  • 778
  • 7
  • 22
  • You didn't post where it's from, but if it's open source, the creators of open source projects can choose to support whatever versions they want, they don't need to give a damn about old compilers. And regarding optimization, you know the entire thing of not optimizing before you need to. Code being more readable is a valid reason, until you identify this as a performance bottleneck. And I doubt it will be. – Erndob Apr 29 '20 at 02:07
  • The question is more generic related to usage of local and static local functions. Someone at ECMA thought there is a need for it, but what need it might be as original intent of the C# 8.0 standard. I don't think their intent was just to allow us to randomly introduce new features for the sake of features. – SoLaR Apr 29 '20 at 02:36
  • I would assume that these local methods would get inlined anyway by the JITter due to their tiny size, and therefore wouldn't use the callstack. I also wouldn't be surprised if the resulting machine code is the same between both approaches. – ckuri Apr 29 '20 at 16:38

1 Answers1

1

I think it's pretty clear from the way C# is developed that readability/ease of use is one of the core pillars for the language. Majority of the new features don't really provide anything truly new, besides giving you an option to write things differently, hopefully more readable/easier.

Almost always you are able to pick something written with a new feature and rewrite it without using it. It doesn't make the new feature invalid.

From design notes related to Local Functions, the intention is stated: https://github.com/dotnet/roslyn/issues/3911

We agree that the scenario is useful. You want a helper function. You are only using it from within a single function, and it likely uses variables and type parameters that are in scope in that containing function. On the other hand, unlike a lambda you don't need it as a first class object, so you don't care to give it a delegate type and allocate an actual delegate object. Also you may want it to be recursive or generic, or to implement it as an iterator.

In your given example, I find the original code more readable. The support for older compilers doesn't matter, if my project targets a new compiler, why would I care about old ones? If I do, where is the end of it? C#7? C#6? 4?

Performance is secondary to readability/maintainability of the code, only when something proves to be a bottleneck for you, you start optimizing it.

So having the options is good.

More advantages to local functions come in when you compare it versus a lambda function, there's a great answer here: Local function vs Lambda C# 7.0

Erndob
  • 2,512
  • 20
  • 32
  • I love how QuickSort was presented using local function, as non-recursive would yield horror code. But reading proposition to the end I share same fears as original author did. I also recognise that original coder only fitter nullable methods to match signature so he could use ?? operator between them and call the resulting method without storing the resulting method in local var or using if's. But still I don't see it, since he had to move the logic into local function, maintainer has to jump all over global function to get that logic. Also (getSize ?? GetSize) at first glance looks like err. – SoLaR Apr 29 '20 at 06:03
  • You can't just take a method out of context and say it's bad. If it's a pattern the team is used to, I would say it's quite readable. In this specific case, getSize is a dynamic thing, the way it's coded now I can read it without the details of how you get the size, only knowing that you do, one way or another. And if I'm interested in details I go to the function definition. Any way, it's confusing what you are asking exactly, is it about the general feature or specifically the code in your example? If it's about the general feature, the design notes explain it. – Erndob Apr 29 '20 at 06:40
  • The problem though is that in this particular case local functions are used for creating delegates, hence have the same disadvantages as inline lambda functions - hidden closures and heap allocations. It's even worse because one has to realize that code like `(getSize ?? GetSize)` creates delegate to the local function with all the associated overhead. – Ivan Stoev Apr 30 '20 at 08:07