Monday, November 6, 2017

The Good, the Bad, and the Ugly of the static keyword

I've been working in a lot of legacy code lately. One of the issues that has come up repeatedly is the misuse and abuse of the "static" keyword in C#. It would be easy for me to knee-jerk and say "static" is just evil, but that would be a disservice to a useful tool that can totally make sense in the right situations. So before I talk about the negatives, let's look at the good stuff!

The Good

Pure Methods

One of the best uses of static is the ability to create pure methods (functions). A pure function meets these requirements (totally stolen from Wikipedia):

  1. The function always evaluates the same result value given the same argument value(s). The function result value cannot depend on any hidden information or state that may change while program execution proceeds or between different executions of the program, nor can it depend on any external input from I/O devices.
  2. Evaluation of the result does not cause any semantically observable side effect or output, such as mutation of mutable objects or output to I/O devices.

Pure functions or methods have several good benefits, including making the program easier to read and reason about, as well as preventing unwanted side-effects. Another tangible in .NET is that static methods can potentially be faster since the run time does not require an object instance to be referenced to run the method. However, I would caution against using static for performance gains unless you can actually measure the difference between using it and not using it. It's too easy to prematurely optimize code that doesn't need it at the cost of readability, or worse, creating something that commits some of the sins I'll list later on. 


Extension Methods

Another great use of static methods is extension methods. The ability to write fluent statements that alter an object, even when you don't own the code for that object, is an incredibly useful feature. While at first glance it does seem to violate the idea of a pure function, extension methods use the "this" keyword to explicitly indicate that a side effect is intended and desired. It's a contract between the caller and the method to do something to the input, so it's all good.

The Bad

Impure Functions with Side Effects

One of the easiest ways to misuse static is to have a function that alters its input arguments. One of the biggest code smells for me is seeing a method that starts with "static void". It means the author is intentionally creating a method full of side effects on objects that the method doesn't own! This is a clear violation of SOLID in terms of encapsulation and the single responsibility principle. It usually indicates some bit of logic that needs to belong to the object itself or some kind of factory or provider class. The best way to address this kind of flaw is to move the logic to a public method of the targeted class or create an extension method. If the logic is creating a series or collection of objects, consider a factory class instead.

The Ugly

Singletons and Static Properties

OK, there may be a few times in your programming career where you find that a singleton of something makes sense. The really important thing is how you implement it, and using the static keyword should not be it. Using static to create a singleton is literally just creating a giant global variable for the scope of the entire application and the programmer using the class can do squat about it. Instead you should use a IoC framework to control the scope of the object as necessary. The application should be able to choose how many of something it needs, and it shouldn't be the decision of the class itself. There are other downsides, not the least of which is that this often makes the class not thread safe. In addition, it is often difficult to unit test singleton classes because unit tests run concurrently may yield unpredictable results (i.e. a result of not being thread safe).

Static properties are another land mine of this idea of using a keyword to create global variables in a class. Not only are they not thread safe, they can result in race conditions where shared state is involved.

Static Dependencies

Probably the worst abuse in static methods (in my opinion) is the inclusion of other static classes and methods being referenced in the static method. This is a clear violation of SOLID and literally hard-wires external dependencies to your class & method, not to mention making them potential global variables. This is also most likely not thread safe (unless you happen to reference only pure functions), so in general is likely to cause you all sorts of headaches at design time and run time. 

Conclusion

There are cases where the static method could genuinely help your code. But in most cases, I see so much confusion and abuse of the keyword that I recommend avoiding it unless you can prove to yourself that a pure function genuinely helps the readability and performance of your code. 




No comments:

Post a Comment