views:

128

answers:

4

ReSharper sometimes hints that I can make some of my random utility methods in my WebForms static. Why would I do this? As far as I can tell, there's no benefit in doing so.. or is there? Am I missing something as far as static members in WebForms goes?

+2  A: 

Resharper suggest to convert methods to static if they don't use any non-static variables or methods from the class.

Benefit could be a minor performance increase (application will use less memory), and there will be one less resharper warning ;)

Fedor Hajdu
+1 FWIW, the built-in Code Analysis engine of Visual Studio (or FxCop) warns about the same thing.
Mark Seemann
Why would the application use less memory?
Eric Lippert
+5  A: 

I wouldn't mind any performance improvement, but what you might like is that static methods have no side effect on the instance. So unless you're having a lot of static state (do you?) this gives away your intention that this method is similar to a function, only looking at the parameters and (optional) returning a result.

For me this is a nice hint when I read someone else's code. I don't worry too much about shared state and can see the flow of information more easily. It's much more constrained in what it can do by declaring it static, which is less to worry about for me, the reader.

Benjamin Podszun
Your belief that static methods are somehow prevented from mutating an instance is false. Static methods can mutate an instance the same as instance methods.
Eric Lippert
+4  A: 

You will get a performance improvement, FxCop rule CA1822 is the same.

From MSDN:

Methods 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

Paolo
The key word there is "for performance-sensitive code". This difference is measured in billionths of a second.
Eric Lippert
+7  A: 

The real reason is not the performance reason -- that will be measured in billionths of a second, if it has any effect at all.

The real reason is that an instance method which makes no use of its instance is logically a design flaw. Suppose I wrote you a method:

class C 
{
  public int DoubleIt(int x, string y, Type z)
  {
    return x * 2;
  }
}

Is this a well-designed method? No. It takes all kinds of information in which it then ignores and does not use to compute the result or execute a side effect. Why force the caller to pass in an unnecessary string and type?

Now, notice that this method also takes in a C, in the form of the "this" passed into the call. That is also ignored. This method should be static, and take one parameter.

A well-designed method takes in exactly the information it needs to compute its results and execute its side effects, no more, no less. Resharper is telling you that you have a design flaw in your code: you have a method that is taking in information that it is ignoring. Either fix the method so that it starts using that information, or stop passing in useless data by making the method static.

Again, the performance concern is a total red herring; you'll never notice a difference that small unless what you're doing takes on the order of a handful of processor cycles. The reason for the warning is to call your attention to a logical design flaw. Getting the program logic right is far more important than shaving off a nanosecond here and there.

Eric Lippert