4

I have a public class called MyClass.cs. It has 3 methods:

 public class MyClass
 {
    public IEnumerable<MyDto> PublicA(bool useCache = true)
    {
    //Call an external resource
    //some code
    }

    public IEnumerable<AnotherDto> PublicB()
    {
    //some code
    var x= MyPrivateMethod(input);
    //some code
    }

    private IEnumerable<AnotherDto> MyPrivateMethod(IEnumerable<SomeDto>)
    {
    //return Mapped data from IEnumerable<SomeDto> to  IEnumerable<AnotherDto>

    }
 }

I use ReSharper as a refactoring tool. It suggests using static for MyPrivateMethod.

private static IEnumerable<AnotherDto> MyPrivateMethod(IEnumerable<SomeDto>)

But what is the usage of this keyword, here? Since the method is private and will not be used in other class that want to use an instance of MyClass.

I tested and found out that, when I use the static keyword for MyPrivateMethod, I can not call any other methods of the class that are NOT private static. But yet I don't know what is the usage? Is there any benefit on storage or time optimization, for example?

Elnaz
  • 2,854
  • 3
  • 29
  • 41
  • 7
    Well it makes it clear that it *isn't* related to any particular instance of `MyClass`. Generally speaking, if a method is logically not related to a specific instance, making it `static` makes that clearer. It also means that you can call it from *other* static methods without having to obtain a reference to an instance. – Jon Skeet Jan 06 '18 at 12:48
  • 1
    _"I tested and found out that, when I use the static keyword for MyPrivateMethod, I can not call any other methods of the class that are NOT private static."_ Then your testing was incomplete. `private static` methods can access any other static methods in the class, whether they are private or not. – JLRishe Jan 06 '18 at 12:51
  • You told, 'making it static makes that clearer.' Do you mean it's just useful for code review and has not any influence on performance? And Also you told ' you can call it from other static methods without having to obtain a reference to an instance'. When the method is private I have no access from other class – Elnaz Jan 06 '18 at 12:55
  • ReSharper sees that you are using `MyPrivateMethod` to share code among other methods of your class, notices that the method does not currently access any non-static methods or fields, and suggests adding `static` to its declaration. It tries to guess your intention to make a "helper method," rather than driving your decision. – Sergey Kalinichenko Jan 06 '18 at 12:57
  • @Elnaz I don't really understand what you just said, but yes, `private static` methods can call any other static method in the class. See here: https://ideone.com/ijLhEr – JLRishe Jan 06 '18 at 13:07
  • Static is Static whatever `public` or `private`, You misunderstood about `static` you can't call any non static method in `static` method.[لینک زیر](http://www.dotnettips.info/post/325/%D8%A8%D8%B1%D8%B1%D8%B3%DB%8C-%D9%88%D8%A7%DA%98%D9%87-%DA%A9%D9%84%DB%8C%D8%AF%DB%8C-static) – Aria Jan 06 '18 at 13:28

1 Answers1

13

As per MSDN

Members that do not access instance data or call instance methods can be marked as static (Shared in Visual Basic). After you mark the methods as static, the compiler will emit nonvirtual call sites to these members. Emitting nonvirtual call sites will prevent a check at runtime for each call that makes sure that the current object pointer is non-null. This can achieve a measurable performance gain for performance-sensitive code. In some cases, the failure to access the current object instance represents a correctness issue.

https://msdn.microsoft.com/en-us/library/ms245046.aspx

One more benefit is calling sequence, when you call an instance method, code generated will push instance of this onto stack as first parameter and rest of the parameters for the method will be pushed onto the stack. So every instance method call requires one more additional stack push for this in along with other method parameters.

If you convert your method to static, static method calls do not require this so one less push operation for CPU. It doesn't seem big benefit for single call.

But if your method will be used very frequently and if you have couple of methods that do not require this then it can save significant CPU time, especially in graphics and scientific calculations.

This is the reason Resharper is suggesting you to change method to static when method does not reference anything that is part of this.

Here is the sample,

    public int Add(int a, int b) {
        return a + b;
    }

    public static int StaticAdd(int a, int b) {
        return a + b;
    }

    public void InstanceAdd() {
        Console.WriteLine(this.Add(3,3));
    }

    public void InstanceAddStatic()
    {
        Console.WriteLine(StaticAdd(3, 3));
    }

This is il generated for calling an instance method in "InstanceAdd"

.method public hidebysig 
    instance void InstanceAdd () cil managed 
{
    // Method begins at RVA 0x2095
    // Code size 16 (0x10)
    .maxstack 8

    IL_0000: nop
    IL_0001: ldarg.0
    IL_0002: ldc.i4.3
    IL_0003: ldc.i4.3
    IL_0004: call instance int32 Temp.MathTest::Add(int32, int32)
    IL_0009: call void [System.Console]System.Console::WriteLine(int32)
    IL_000e: nop
    IL_000f: ret
} // end of method MathTest::InstanceAdd

and this is the il generated for instance method in "StaticAdd"

.method public hidebysig 
    instance void InstanceAddStatic () cil managed 
{
    // Method begins at RVA 0x20a6
    // Code size 15 (0xf)
    .maxstack 8

    IL_0000: nop
    IL_0001: ldc.i4.3
    IL_0002: ldc.i4.3
    IL_0003: call int32 Temp.MathTest::StaticAdd(int32, int32)
    IL_0008: call void [System.Console]System.Console::WriteLine(int32)
    IL_000d: nop
    IL_000e: ret
} // end of method MathTest::InstanceAddStatic

If you look at "StaticAdd", there is no ldarg.0, which is this. For every method call, there will always be ldarg.0 as first instruction and then rest of the parameters will follow.

Akash Kava
  • 39,066
  • 20
  • 121
  • 167
  • Thanks. Do you have any link as reference to help me understanding it more? Particularly, about your first paragraph. – Elnaz Jan 06 '18 at 13:08
  • 1
    I'm pretty sure the reason Resharper recommends it has nothing to do with the CPU savings from not having `this` on the stack. – JLRishe Jan 06 '18 at 13:09
  • @JLRishe you mean to say there is no benefit on CPU saving using static method? – Akash Kava Jan 06 '18 at 13:12
  • @AkashKava No, I mean to say I don't think that the situation you described is the _reason_ Resharper suggests it, at least not the only reason (you claimed that was Resharper's reason in the last paragraph). Do you have some citation that explains their reasons for recommending it? – JLRishe Jan 06 '18 at 13:17
  • @JLRishe https://msdn.microsoft.com/en-us/library/ms245046.aspx check rule description, however this is MSDN but I am sure, Resharper guys are following this as rule set. – Akash Kava Jan 06 '18 at 13:26
  • @AkashKava The explanation on that MSDN page is completely unrelated to the reason given in your answer. – JLRishe Jan 06 '18 at 13:27
  • @JLRishe yes I agree, I have updated my answer and I have also added il examples as an added benefit. – Akash Kava Jan 06 '18 at 13:28