CodeRush suggests making a method static if it has parameters, agree or disagree?

Question

I use Developer Express's CodeRush product which has a feature called Code Issues which makes suggestions to optimize your code. I've noticed that if you have a method that has parameters it will always suggest making this method static. In the spirit of trying to write the best code and optimized which I assume is what DevExpress is trying to help us do, I've heard mixed opinions on whether making the method static is in fact wise.

What is your opinion on when a method should be static? Is there any benefit to doing this? Impact? I don't see anything wrong with it as it is requiring parameters to run the method so it is not something that would be an issue across multiple users/uses.

Good or Bad?

Thanks.

Answer 1

Whether or not a method has parameters has nothing to do with it. The only consideration is whether the method attempts to access any non-static (i.e., instance-based ) members of the class, or if some other static member in the class calls it....

For example

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

can be static - it does not access any instance members of its container class, even though it has parameters

But in the following class, PrintHello() cannot be declared as static since it accesses the instance-based field useCount, even though it has no parameters.

public class myClass
{
    private int useCount = 0;

    private void PrintHello()
    { 
        useCount = useCount + 1;
        Console.Write("Hello");
    }
}

Answer 2

I usually stay a way from using static methods, because they can be difficult to use with unit testing and dependency injection. (I know about typemock). Only if the method is tightly contained would I consider using a static method.

Answer 3

As others have indicated, the presence of parameters is not a consideration. The hint is present when the method does not access any instance members.

When is this useful?

When a candidate method requires no state to perform it's function, it should not be required that the caller of this function possess or create an instance of the parent class.

By removing this requirement, the calling code will surface errors (or in some languages warnings) indicating that the method in question "cannot be accessed with an instance reference" requiring a Type Reference instead.

When the calling statement is rewritten to use a Type reference, it may be discovered that the original instances of the type is not required and code may be refactored further to eliminate he creation of same.

The elimination of this instance type will save cpu and memory.

The calling method (or it's own callers) will likely be easier to read since instantiation code is not needed for the method in question.

Additionally if instantiation code is missing, this will add to the readability of one or more methods.

For example the System.Math class is static and therefore filled with static functions. Calling code for these functions would be less readable if it had to instanciate an instance of the math class prior to execution.

Answer 4

Personally, I'm of two minds on the subject.

On the one hand, one could argue that the method should be made static because it doesn't access any instance members. And while there's some truth to that, there are several reasons I'm not always in a big hurry to run out and do that:

1. Namespace pollution. If we get into this habit, it starts to smack of the "good ol' days" when everything was lumped into global modules and you could just invoke functions willy nilly. I shudder at the thought. While static methods are, admittedly, organized into classes and namespaces, they are still not instance specific, and they just smack of global functions to me. They make my toes curl like the sound of fingernails on a chalk board.

2. Just because a method doesn't access instance variables today doesn't mean it won't tomorrow. And refactoring it to do so is a breaking change. Making a method static isn't a decision that should be made lightly. You do it because you KNOW the method is intended to be used that way, not because a tool noticed that today you aren't using instance variables.

Now, having said that, I do believe that static methods serve a very useful purpose and, when used properly, solve myriads of problems splendidly. I'll use them when I intend to and for very specific purposes that frequently have nothing whatsoever to do with whether or not Resharper thinks I should do it.

I haven't seen anything that states that the application will perform more efficiently if a method is marked static. I might be inclined to believe that static classes degrade performance, since they exist for the lifetime of the application, and any resources they allocate won't be released until the application goes out of scope. (On the other hand, depending on implementation, this might result in a performance boost.)

Ultimately, the performance issue comes down to implementation.

Answer 5

I'd personally make them static. It feels more right that if something doesn't work on the instance it shouldn't be a member on the instance (but on the class).

For example an add method that takes two parameters and adds them and returns the result will need nothing more to process itself so can be static. If you left it as an instance method it might imply that it will make a difference which isntance you run it on which it clearly won't!

I don't imagine there is any advantage in terms of efficiency (though I haven't looked into it closely), just in readability of code.