The reasons given are:
- They can't be tested
- They can't be mocked
- They make the code more difficult to understand
- They have hidden side effects
Every time I see articles like this I figure I must have arrived in Opposite Land™ because this advice is in complete contradiction to my two decades of programming experience.
Static functions are awesome because:
- They are easier to test
- They are just as easy to mock
- They make the code much clearer
- They have no side effects
The disagreement comes down to what a static function represents..
This is the typical example of why a static function is hard to mock:
public class Foo {
public void doSomething() {
// stuff...
int result = Utils.doSomething(parameter);
// more stuff..
}
}
There's no easy way to mock Utils.doSomething(..). Apparently, PowerMock can mock static methods but let's assume we're not using any special voodoo.
If we take our original example and replace the static method with an object instantiation:
public class Foo {
public void doSomethingFancy() {
// stuff...
int result = new Utils().doSomething(parameter);
// more stuff..
}
}
..we're still not mockable. Is it because constructors are evil? No, constructors are not evil. Creating a new object at this point in the code is.. well, not evil. .Let's just say it's not conducive to mocking. What if we do this?
public class Foo {
private final Utils mUtils;
public Foo(Utils utils) {
mUtils = utils;
}
public void doSomethingFancy() {
// stuff...
int result = mUtils.doSomething(parameter);
// more stuff..
}
}
Great! Now we have code that's mockable, except we now have code that is misleading and coupled .
First off, we have an instantiable Utils class. This violates a common naming convention of the Java community. The Java community adds "Utils" or "Utilities" to signal that a class contains a bunch of static methods. This Utils class is meant to be instantiated and then used like a collection of static methods. By my calculations the silliness quotient outweighs the added mockability by 34.23%.
Secondly, it's not clear which methods of our now badly named Utils class are being used and which methods are just along for the ride. In real code, where the Foo class might be 500 lines long, we would have to go poking around and try to understand how the Foo class was using the Utils class. Which methods do we actually need to mock and which are noise? Why not use a custom interface instead? This is called the Strategy design pattern. Here's what the Foo class would look like with a Strategy pattern:
public class Foo {
private final DoSomethingStrategy mStrategy;
public Foo(DoSomethingStrategy strategy) {
mStrategy = utils;
}
public void doSomethingFancy() {
// stuff...
int result = mStrategy.doSomething(parameter);
// more stuff..
}
// custom interface defines only what we need!
public interface DoSomethingStrategy {
int doSomething(String parameter);
}
}
... and this is how I would have instantiated the Foo class in Java 1.7:
Foo foo = new Foo(
new DoSomethingStrategy() {
public int doSomething(String parameter) {
return Utils.doSomething(parameter);
}
} );
Thankfully Java 1.8 has gotten rid of all this pointless extra syntax and it becomes:
Foo foo = new Foo(Utils::doSomething);
We're passing a static function to a class as a strategy!
What if you have a Strategy with multiple methods? You can either define two Strategy interfaces (which is fine) or bite the bullet and do it the long way:
public DoSomethingStrategy {
int doSomething(String parameter);
int doSomethingElse(Long parameter);
}
Foo foo = new Foo(
new DoSomethingStrategy() {
public int doSomething(String parameter) {
return Utils.doSomething(parameter);
}
public long doSomething(long parameter) {
return CustomMathUtils.calculateSomething(parameter);
}
} );
It doesn't matter if your interface implementation is in a static method or in a class, you should always apply the Strategy pattern when implementing mockable Strategies - the cost is so low and the code readability is so much better.
Before I go I would like to talk about the advantages of static methods.
Ideally your static functions should be Pure Functions. If you static methods aren't pure functions then you're evil. They don't have to be devoid of all side effect like printing out a log trace but the closer the better. The only dependencies in static function should be listed as parameters. This means that for any given set of parameters the function will always return the same result (aka: idempotent). No weird side effects or black magic shenanigans allowed. Once you're thinking in pure functions you're on your way to Functional Programming design patterns. Each Pure Function is by definition a testable chunk.
Let's say you're refactoring a BigClass. It's over 1000 lines and is a mix of miscellaneous stuff. Typically, in these cases, there's some fairly self contained code that implements fancy algorithms along with some state-full code that implements the objects interface. Where to start? First thing I do in this case is to try and find the lurking stateless algorithm code and pull that out. The way you do that is you look for the hidden static functions. These are functions that don't touch the object's meber variables. Code like this:
private void foo() {
mMemeberVar = 7;
mOtherMemeberVar = calculateStuff(mOtherMemeberVar);
mCancelled = false;
}
Would be a statefull since it uses object member variables (that's the "m" prefix means). calculateStuff(..) probably doesn't access object member variables but we'd have to poke through a big pile of code to make sure. Either that or we could mark it as "static" and see what breaks.
At my day job I work on a team with 20 other developers and we've more-or-less followed the practice making any method that doesn't rely on object member variable static. The rule is, if it can be static then mark it as static. This is a Godsend when you're trying to refactor some BigClass and you see a bunch of static methods that you can pull out into a private BigClassUtils. Hurray 300 lines of static methods that I no longer have to wade through to find the good stuff.
Since we've pulled out a big blob of Pure Functions from BigClass why don't we add unit tests for them? I mean, they're all stand alone chunks of code otherwise they couldn't be static methods. So now we can unit test these static methods without worrying about weird dependencies or side effects. Not only that but calls to BigClassUtils are good candidates for the BigClassStrategy interface as well.
Still worried about static methods calling each other at nausenum? Good! Because you can fix that using the Strategy pattern too:
static void List foo( List fromList ,
Function transform ) {
ArrayList result = new ArrayList();
foreach( item : fromList ) {
result.add(transform.apply(item));
}
return result:
}
In this case the param "transform" is action like a Strategy.
You call foo() like this:
List integers = foo(strings, Utils::doSomething);
We're passing static functions to static functions!
Before I continue I should mention that while static methods are awesome, static class members are evil. Ideally the static functions should be pure functions although you don't need to get eliminate all side effects. Just the ones that mutate program structure.
What you should do, is go into your compiler settings and tell the compiler to emit a warning on every static member variable. There are very few cases where it makes sense to have static member variables and in all cases they should be wrapped in a singleton. Ideally a Singleton with no public static getInstance().
I can't tell you the amount of pain Singletons have caused. If you're on a large team and create a singleton it's very hard to stop team members from just reaching into the singleton bag and accessing it directly.That means you're in the middle of testing some code and suddenly the default Singleton implementation has come to life and it's causing some of your tests to access the production server. doh.
Sure, you can mock the singleton but that is a temporary fix at best. At some point there will be code that needs the default singleton implementation or forgets to set the implementation or leaks some resource or some such issue and you'll spent your week-end trying to figure why this only breaks on the build server. If you parametrize all your dependencies it's far easier to debug. Everything is right there in front of you. You'll have fairly large constructors but I always say "that's what coupling looks like". If you want small constructors stop coupling things together (alternatively use any number of techniques to add layers between your class and the stuff you're using, potentially reducing your argument count to 1 - your Strategy).
I hope to have shown that it's not static functions that are evil it's badly written static functions that are evil. Static methods with static state or not using Strategies appropriately. Static functions should represent Pure Functions. Static functions are just fine, if you're not writing Pure Functions it's because you're evil. It's similar to "the force" in star wars; you must always try and stay on the light side of static functions or you'll have your hand cut off by your dad.
And now, here is the picture of a late model Sherman tank:
Posts
No comments:
Post a Comment