C#: ReSharper complains when method can be static, but isn't

Question

Why does ReSharper complain when a method can become static, but is not?

Is it because only one instance of a static method is created (on the type) and thus save on performance?

Answer 1

You don't have to push "this" onto the function's stack for a static method. That's another reason it's cheaper.

Answer 2

No (zero) instances of the class need to be created to use the method if it is declared as static... that saves cpu cycles necessary for construction processing, heap space, and cpu cycles by the Garbage collector in reclaiming the object from the heap...

Also, your question, as it it written

" ... only one instance of a static method is created (on the type)... "

implies that for an instance method, the code for the method is repeated for each instance of the class that is created. That is not true. No matter how many instances you create for any type, the code for the methods is loaded intto memory only once. The object stored on the heap for each instance only stores the type's "State", (non-static fields & a few misc tracking variables).

Answer 3

It's not a complaint, it's just advice.

Answer 4

Please understand that sometimes ReSharper can show false alarms. Well, simply send the team your feedback can help them improve this great product.

Regards,

Answer 5

From the FxCop documentation for the same warning (emphasis added):

"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 non-virtual call sites to these members. Emitting non-virtual call sites will prevent a check at runtime for each call that ensures that the current object pointer is non-null. This can result in a measurable performance gain for performance-sensitive code. In some cases, the failure to access the current object instance represents a correctness issue."

Answer 6

I find that comment very useful as it points out two important things:

1. It makes me ask myself if the method in question should actually be part of the type or not. Since it doesn't use any instance data, you should at least consider if it could be moved to its own type. Is it an integral part of the type, or is it really a general purpose utility method?

2. If it does make sense to keep the method on the specific type, there's a potential performance gain as the compiler will emit different code for a static method.

Answer 7

For me, the greatest benefit of this ReSharper advice (you can set it as a warning, suggestion, or hint). Is that it encourages me to make as many methods static as possible. This is a Good Thing, since a static method has no direct dependency on the class it is a member of. This means that it can easily be moved to another class as a static member.

Another neat trick with statics in ReSharper is to make a set of related methods static by using the "Make Method Static" refactoring. This will move some of the dependencies into method parameters. When you look at this set of methods later, you may find they all access a particular object of a particular type. You can then use the "Make method non-static" refactoring, and specify that object as being the new this pointer. That moves your method into the other class.

From this:

internal class ClassA
{
    public ClassB Property { get; set; }

    public int Method()
    {
        var classB = Property;
        return classB.Property1 + classB.Property2;
    }
}

internal class ClassB
{
    public int Property1 { get; set; }
    public int Property2 { get; set; }
}

to this:

    public static int Method(ClassB property)
    {
        var classB = property;
        return classB.Property1 + classB.Property2;
    }

to this:

internal class ClassA
{
    public ClassB Property { get; set; }
}

internal class ClassB
{
    public int Property1 { get; set; }
    public int Property2 { get; set; }

    public int Method()
    {
        return Property1 + Property2;
    }
}

Answer 8

Very good debate on that subject here (SO). I am in the camp of if-it-can-be-made-static-make-it-static. I believe this because of the notion of why one would have an instance method that does not use any instance data. Is it truly an instance method in that case or is it actually a class method?