Article: “Static methods are a code smell”

I wrote an article a while back for TechTarget / SearchWinDevelopment about static methods being a code smell.  It got published a few weeks ago and here’s a link to read it.

Static methods are a code smell
by Ben Day
02.26.2010

Ever heard of code smells? A code "smell" is a way of saying that there is potentially a problem with your code. It doesn't mean that you've definitely made a mistake or that you're definitely wrong for doing something wrong, it just means that you *might* be doing something wrong. At the moment, Wikipedia says that it is "any symptom in the source code of a program that possibly indicates a deeper problem."

One of the common smells that I come across is static method and variables. (In VB.NET these are known as "Shared" methods.) If you mark a method as static, this means that the method now belongs to the type rather than an instance.

(read the article)

-Ben

posted @ Monday, March 15, 2010 11:51 AM

Print

Comments on this entry:

# 

Left by Jason Haley at 3/16/2010 6:46 AM
Gravatar
Interesting Finds: March 16, 2010

# re: Article: “Static methods are a code smell”

Left by Wayne B at 3/16/2010 10:21 AM
Gravatar
This article smells. It puts forth a single statement without so much as an example or an explanation.

# re: Article: “Static methods are a code smell”

Left by Benjamin Day at 3/16/2010 10:24 AM
Gravatar
Thanks for the feedback, Wayne. :)

# re: Article: “Static methods are a code smell”

Left by Merill Fernando at 3/16/2010 7:11 PM
Gravatar
I would disagree.

Resharper has a rule to suggest changing a method to static if it does not use any instance variables.

What you fail to mention is that your arguments on inheritance etc refer to public methods only. They don't apply to private methods.

With private methods IMO it is much better to make methods static when possible. This follows the SOLID and KISS principle and increases code isolation.

You should be careful about making overarching statements like the one you just did, without thinking about the exceptions.

# Markus Tamm » Blog Archive » Links 17.03.2010

Left by at 3/17/2010 4:52 AM
Gravatar
Markus Tamm » Blog Archive » Links 17.03.2010

# re: Article: “Static methods are a code smell”

Left by Benjamin Day at 3/17/2010 8:01 AM
Gravatar
Merill,

I think that you're right. Private methods that don't use any instance variables would actually be acceptable candidates for conversion to static methods.

Keep in mind that a code "smell" doesn't immediately mean that it's wrong. A smell is a indication that that something might not be correct.

For the case of private instance methods, I'm not sure that I would convert them to static just because I *could* convert them to static.

1. A private static method in a non-static class implies "utility method" to me. Private static methods are extremely difficult to unit test. I like to move utility methods off to separate static classes and, once they get moved, I'm probably going to make that method "public static" for unit testing purposes.

2. Just because it's easy to change using tools like ReSharper and CodeRush, does that have to mean that you *must* change these methods to static? I understand that there's some performance gain in .NET for doing this but will you ever *really* see that performance improvement? In most applications, I think that the performance improvement is going to be so small as to be nearly meaningless. If you aren't seeing a performance penalty with a private instance variable, I think that I'd very likely err on the side of maintainability and skip the static refactoring.

My $0.02.

-Ben

# re: Article: “Static methods are a code smell”

Left by Merill Fernando at 3/17/2010 5:08 PM
Gravatar
Ben thanks for the reply.

I guess it boils down to the 'Unit testing of private methods' argument. I'm in the don't test private methods camp as I've seen the brittleness of unit tests when you do so.

As for the utility methods my rule is to keep them private to whichever class is using it and only re-factor it out when it is really needed by another class. If not why even bother making it public and increase your surface area.

Your comment:



 (will not be displayed)


 
 
 
Please add 6 and 4 and type the answer here:
 

Live Comment Preview:

 
«September»
SunMonTueWedThuFriSat
2930311234
567891011
12131415161718
19202122232425
262728293012
3456789