Should C# methods that *can* be static be static?

Question

Should C# methods that can be static be static?

We were discussing this today and I'm kind of on the fence. Imagine you have a long method that you refactor a few lines out of. The new method probably takes a few local variables from the parent method and returns a value. This means it could be static.

The question is: should it be static? It's not static by design or choice, simply by its nature in that it doesn't reference any instance values.

Answer 1

I suggest that the best way to think about it is this: If you need a class method that needs to be called when no instances of the class are instantioated, or maintains some kind of global state, then static is a good idea. But in general, I suggest you should prefer making members non-static.

Answer 2

I would not make it a public static member of that class. The reason is that making it public static is saying something about the class' type: not only that "this type knows how to do this behavior", but also "it is the responsibility of this type to perform this behavior." And odds are the behavior no longer has any real relationship with the larger type.

That doesn't mean I wouldn't make it static at all, though. Ask yourself this: could the new method logically belong elsewhere? If you can answer "yes" to that, you probably do want to make it static (and move it as well). Even if that's not true, you could still make it static. Just don't mark it public.

As a matter of convenience, you could at least mark it internal. This typically avoids needing to move the method if you don't have easy access to a more appropriate type, but still leaves it accessible where needed in a way that it won't show up as part of the public interface to users of your class.

Answer 3

If you were able to refactor a few lines out and the resulting method could be static, it is probably an indication that the lines you pulled out of that method don't belong in the containing class at all, and you should consider moving them into their own class.

Answer 4

It depends. There are really 2 types of static methods:

1. Methods that are static because they CAN be
2. Methods that are static because they HAVE to be

In a small to medium size code base you can really treat the two methods interchangeably.

If you have a method that is in the first category (can-be-static), and you need to change it to access class state, it's relatively straight forward to figure out if it's possible to turn the static method into a instance method.

In a large code base, however, the sheer number of call sites might make searching to see if it's possible to convert a static method to a non static one too costly. Many times people will see the number of calls, and say "ok... I better not change this method, but instead create a new one that does what I need".

That can result in either:

1. A lot of code duplication
2. An explosion in the number of method arguments

Both of those things are bad.

So, my advice would be that if you have a code base over 200K LOC, that I would only make methods static if they are must-be-static methods.

The refactoring from non-static to static is relatively easy (just add a keyword), so if you want to make a can-be-static into an actual static later (when you need it's functionality outside of an instance) then you can. However, the inverse refactoring, turning a can-be-static into a instance method is MUCH more expensive.

With large code bases it's better to error on the side of ease of extension, rather than on the side of idealogical purity.

So, for big projects don't make things static unless you need them to be. For small projects, just do what ever you like best.

Answer 5

I think it would make it a bit more readable if you marked it as static...Then someone who comes along would know that it doesn't reference any instance variables without having to read the entire function...

Answer 6

Personally, I'm a great fan of statelessness. Does your method need access to the state of the class? If the answer is no (and it is probably no, otherwise you wouldn't consider making it a static method), then yeah, go for it.

No access to state is less headache. Just as it is a good idea to hide private members that are not needed by other classes, it is a good idea to hide the state from members that don't need it. Reduced access can mean less bugs. Also, it makes threading easier as it is much easier to keep static members thread-safe. There is also a performance consideration as the runtime does not need to pass a reference to this as a parameter for static methods.

Of course the downside is that if you ever find that your previously static method will have to access the state for some reason, then you have to change it. Now I understand that this can be a problem for public APIs so if this is a public method in a public class, then perhaps you should think about the implications of this a bit. Still, I've never faced a situtation in the real world where this actually caused a problem, but maybe I'm just lucky.

So yeah, go for it, by all means.