16

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...

Nobody
  • 4,731
  • 7
  • 36
  • 65
  • possible duplicate of [Static vs. non-static method](http://stackoverflow.com/questions/1184701/static-vs-non-static-method) – Fredrik Mörk Sep 13 '10 at 10:42
  • If the method doesn't require instance specific data you can make it static. – halfdan Sep 13 '10 at 10:42
  • I know halfdan, but I'm asking - Should I? – Nobody Sep 13 '10 at 10:44
  • 1
    This is an interesting question. I can't answer because I have not enough XP for that. However, here is an interesting link which argue where use or not the "static" keyword : http://codebetter.com/blogs/patricksmacchia/archive/2009/11/16/back-to-basics-usage-of-static.aspx – Florian Sep 13 '10 at 11:26

5 Answers5

18

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).

8protons
  • 3,591
  • 5
  • 32
  • 67
Michael Shimmins
  • 19,961
  • 7
  • 57
  • 90
4

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.

Alex Humphrey
  • 6,099
  • 4
  • 26
  • 41
  • You say that if the method is only useful within the class, then don't make it static. Isn't it also performance gained by making a method static? Or else what would be the point of having a `private static` method (which is possible, and actually suggested by ReSharper)? – awe Mar 12 '12 at 12:46
  • @awe - you're correct that calling static methods should be quicker. However, I think for this specific example that calling `someClassInstance.SomeMethod()` is better than `SomeClass.SomeMethod(someClassInstance.AProperty)`. I don't think the suggested refactor in the question is the best thing to do - in many other cases it is. Does ReSharper suggest that refactor for the example above? It would surprise me if it did as the method accesses an instance property. You could go the whole hog and write `static SomeMethod(SomeClass this)` for *all* your non-virtual instance methods if you wanted. – Alex Humphrey Mar 13 '12 at 08:59
  • I agree with you in this case, but you formulated it in a way that was not clear to me. I mean in the example where a method is only useful within the context of the class, but technically does not use any of the class members. The method is really just some sort of utility function, but no other code than within the class need this functionality. Resharper suggest refactor if your method does not use any non-static members or methods in your class. I noticed this in a private method I created that ReSharper suggested to make static. – awe Mar 13 '12 at 11:02
  • @awe - For your example I agree with making the method static and leaving it in the class. In fact that's a fairly common refactor for me. – Alex Humphrey Mar 13 '12 at 11:49
  • A note on performance. A static function and a non-virtual method have exactly the same per-call performance characteristics. What makes a static function faster is that you don't need to instantiate an instance of an object before you call it. Which means you have one less object to GC. – Jonathan Allen Aug 14 '16 at 15:02
2

Static methods make sense, if you should be able to call them without creating an object of the class before. In Java, for example, the Math-Class contains only static methods, because it wouldn't make much sense to instanciate a Math-Class only to do mathematical operations on other objects.

Most of the time it's better to avoid static methods. You should get familiar with object oriented programming - there are lots of good resources out there, explaining all the concepts like static methods, etc.

cyphorious
  • 809
  • 3
  • 7
  • 19
1

No. Static is evil. It tightly couples the caller to the used class and makes it hard to test.

Sjoerd
  • 74,049
  • 16
  • 131
  • 175
  • 2
    Microsoft's Code Analysis tool is always suggesting that I should make methods static if there are no instance dependencies. Is this bad advice? – Polyfun Sep 13 '10 at 10:48
  • @ShellShock Might be. If you have many such methods, you probably violate the OOP principles. A method of an object should (almost) always work with the data associated to the object. – Karel Petranek Sep 13 '10 at 10:49
  • @dark_charlie: So what you say is that if you can have a method as static, you should restructure to put it out in a static utility class instead? What if the method is logically tied to the class, but technically not using any members? – awe Mar 12 '12 at 12:26
  • 1
    The correct answer is of course "it depends". There are scenarios where the static method belongs to the class. But those are quite rare and the general rule is to try to avoid static methods. – Karel Petranek Mar 12 '12 at 16:46
  • 2
    Uh, `Math.Max` is not hard to test. – Jonathan Allen Aug 14 '16 at 15:04
1

I think it will depend on the way you want to use the methods. Using a static method is okay if it is used as a common method over a few instances of the class.

For the sake of an example, say you have a string class and two strings A and B. To compare A and B, you can either have a A.CompareTo(B) method or String.Compare(A, B) method.

Please correct me if I am wrong.

Rahul
  • 1,866
  • 17
  • 32
  • I think `XDocument.Parse` would be a better example. There wouldn't be much point in creating an XDocument just to use its parse method to create the XDocument that you actually want. – Jonathan Allen Aug 14 '16 at 15:07