Should I always make my methods static where possible?

Question

I have often pondered this one... its probably an idiot question but here goes.

Say I have this class:

public class SomeClass
{
    public int AProperty { get; set; }

    public void SomeMethod()
    {
        DoStuff(AProperty);
    }
}

Is there any advantage to doing this:

public class SomeClass
{
    public int AProperty { get; set; }

    public static void SomeMethod(int arg)
    {
        DoStuff(arg);
    }
}

The only advantage that is obvious is that I can now access SomeMethod directly.

So is it good practice to make these kind of methods static where a little refactoring will allow or is it a waste of my time?

EDIT: I forgot to mention (and ShellShock's comment reminded me) that the reason I ask is that I use ReSharper and it always makes suggestions that 'Method X can be made static' and so on...

Answer 1

Static isn't evil. Static is evil if used incorrectly, like many parts of our programming toolkit.

Static can be very advantageous. As the accepted answer here points out, static can have a potential speed improvement.

As a general rule if the method isn't using any fields of the class then its a good time to evaluate its function, however ultimately utility methods that can be called without instantiating an object can often be useful. For instance the DirectoryInformation and FileInformation classes contain useful static methods.

Edit

Feel obligated to point out that it does make mocking a lot harder but it is still definitely testable.

It just means you need to think harder about where static methods go, so that you can always test them without needing to rely on a mock/stub. (ie: don't put them on your DTO that requires a persistent connection to the database).

Answer 2

I'll attempt to answer your specific question involving the code sample you provided.

If SomeMethod is only useful in the class it is declared in, I would avoid the static conversion and leave it as an instance method.

If SomeMethod is useful outside of the class it is in, then factor it out of the class. This may be as a static method in a static utility class somewhere. To make it testable, ensure that all its dependencies are passed in to it as arguments. If it has loads of dependencies, you might want to review the design and figure out exactly what it's supposed to be doing - it might be better as an instance method in one of the classes you're passing in to it.

Some people say that static is evil. This is generally because of the pitfalls that mutable static state provides, where variables hang around from the point a static constructor is called to the tear down of an app domain, changing in between. Code reliant on that state can behave unpredictably and testing can become horrendous. However, there is absolutely nothing wrong with a static method which does not reference mutable static state.

For a (very simple) example where a static is evil, but can be converted to a non-evil version, imagine a function that calculates someone's age:

static TimeSpan CalcAge(DateTime dob) { return DateTime.Now - dob; }

Is that testable? The answer is no. It relies on the massively volatile static state that is DateTime.Now. You're not guaranteed the same output for the same input every time. To make it more test friendly:

static TimeSpan CalcAge(DateTime dob, DateTime now) { return now - dob; }

Now all the values the function relies on are passed in, and it's fully testable. The same input will get you the same output.