Have you ever seen or written static methods in your company’s project? I’m sure that some of you did, because I often fell into static classes when I dug into most of the entreprises' code I visited. The question is: Do you think it’s a good practice?
Just to make things clear before I start writing about this, Static and Shared are the same things. The former is used in Java / C#, while the latter is employed in VB. Therefore, don’t mind if I alternate between the two in this post.
Here’s what MSDN has to say about shared methods and classes:
Use a static class as a unit of organization for methods not associated with particular objects. Also, a static class can make your implementation simpler and faster because you do not have to create an object in order to call its methods.
I will summarize and complete the thoughts of Mark S. Rasmussen on this subject.
The pros that people often tell me about shared methods is that:
- They make great helper methods in utility classes
- They are more concise than code creating an instance that would be of no use afterward
- There is no need to create an instance when there is no state
- They are great for small projects without any architectural need and no possible future maintenance
Now, if you like shared methods, please pay attention to the cons of using static classes and methods.
It’s impossible to make and maintain unit tests with any classes that use static stuff. Sure, we don’t really have to bother if only static classes like Math are used, since they are standardised tools. But we should not depend on static classes that we (or someone else) could possibly change one day.
There is just no way to isolate a class from static methods. Since you can’t make interfaces implemented by static classes, you can’t mock them. Imagine that this static thing creates and use 3 new classes and call the GUI or even a database… A single unit test will not be fast, will not be unitary and will fail randomly. As a side subject, go read Martin Fowler’s post about eradicating Non-Determinism in tests.
Mark S. Rasmussen explains it wonderfully:
Say we have the method
UtilityClass.SomeMethodthat happily buzzes along. Suddenly we need to change the functionality slightly. Most of the functionality is the same, but we have to change a couple of parts nonetheless. Had it not been a static method, we could make a derivate class and change the method contents as needed. As it’s a static method, we can’t. Sure, if we just need to add functionality either before or after the old method, we can create a new class and call the old one inside of it - but that’s just gross.
Oh, cool! A shared method that will be really helpful. Nice, here’s another one. I shall put them in the same shared class. Oh but wait, how should I name this class? I don’t know really much, but both methods are doing stuff for the validation of html query strings. Anyway, let’s call it “ValidationHelper”!
Shared classes tend to be hard to name, and because of that, one will inevitably name it a generic prefix (like Validation) and append the horrible “Helper” at the end. Do you see or smell smoke? Let me give you a hint: “Hey, I’ve made another static method that validate the state of a class. Now, where should I put this static method?”
Shared stuff is hard to name, so it will receive a generic name and therefore will get bigger and bigger over time. There, you will see appear classes that are blobs of unrelated shared methods. And then optional parameters (or methods with same name but different parameters) will creep in.
If you absolutely have to make shared classes (please don’t), make sure that you don’t maintain shared attributes with it. This would mean that you have global variables, which will be a real nightmare to debug. And don’t worry, you WILL have to debug, since there’s no way to have a safety net of unit tests and because complexity increase exponentially with global states. That, and you killed your architecture with coupling… A big chainsaw strike in the backbone of your project. Ever wished to push your scalable application to a cloud? Well, you can just put this task in the Will-Never-Be-Able-To-Do list.
A Rule of Thumb
OK. Now that we know that shared stuffs are often evil, how do I get rid of them in this huge enterprise project? It’s already used by over 9000 classes.
One possibility is to use a Singleton. WAIT. I know they are just as bad. No problem is solved. Still, when you are unable to make the switch to a regular class in one step, it’s a good option. But make sure to use the unit-testable version of it, the service locator.
Another (better, if you ask me) solution would be to create a non-static wrapper class that transfer the calls to the shared methods. Progressively, make every class that used the shared methods use this instantiable class instead. Once no class but your wrapper uses the static class, refactor the code by deleting the static calls and by implementing the code in a non-static way. Or leave it this way and mock the non-static class.
In order to avoid failing or feeling discouraged undergoing the cumbersome task, find ways to take small bites. Don’t Big-Bang-Refactor. Been there, tried that, and failed before you.