Why is using static helper methods in Java bad?

Question

I'm asking because I'm trying to use a mocking framework (Mockito) which does not allow you to mock static methods. Looking into it I've found quite a few blog posts saying that you should have as few static methods as possible, but I'm having difficulty wrapping my head around why. Specifically why methods that don't modify the global state and are basically helper methods. For instance I have a class called ApiCaller that has several static methods. One of the static method's purpose is to execute an HTTP call, deal with any custom issues our server might have returned (ex. user not logged in) and return the response. To simplify, something like:

public class ApiCaller {
...
   public static String makeHttpCall(Url url) {
        // Performs logic to retrieve response and deal with custom server errors
        ...
        return response;
   }
}

To use this all I have to do is call ApiCaller.makeHttpCall(url) Now I could easily make this a non static method like:

public class ApiCaller {
...
   public String makeHttpCall(Url url) {
        // Performs logic to retrieve response and deal with custom server errors
        ...
        return response;
   }
}

and then to use this method call new ApiCaller().makeHttpCall() but this just seems like extra overhead. Can anyone explain why this is bad and if there is a better solution to making the methods non static (other than just removing the keyword) so that I can stub out these methods using the mocking framework?

Thanks!

Answer 1

The problem with static methods is they're very hard to fake when they're not relevant to the system you're trying to test. Imagine this code:

public void systemUnderTest() {
    Log.connectToDatabaseForAuditing();
    doLogicYouWantToTest();
}

The connectToDatabaseForAuditing() method is static. You don't care what this method does for the test you want to write. But, to test this code now you need an available database.

If it were not static the code would look like this:

private Logger log; //instantiate in a setter AKA dependency injection/inversion of control

public void systemUnderTest() {
    log.connectToDatabaseForAuditing();
    doLogicYouWantToTest();
}

And your test would be trivial to write without a database now:

@Before
public void setUp() {
    YourClass yourClass = new YourClass();
    yourClass.setLog(new NoOpLogger());

}

//.. your tests

Imagine trying to do that when the method is static. I can't really think of a way except for modifying the logger to have a static variable called inTestMode that you set to true in the setUp() to make sure it doesn't connect to a database.

Answer 2

It is less modular. Instead you should define an interface ApiCaller with an instance method makeHttpCall() so that you can define separate implementations in the future.

In the very least you will always have 2 implementations of an interface, the original and the mocked version.

(Note: there are some mocking frameworks that allow you to mock static methods)

As an addendum, while this may not be the case in your specific application, typically the use of static methods is indicative of a larger design oversight. Designing for modularity and reuseability should be prevalent throughout your application, because even though you don't need it right now you may need it in the future, and it's much harder and much more time consuming to change things after the fact.

Answer 3

PRIVATE Static helper methods are not bad, in fact, they are actually preferred at the large corporation where I work. And I use Mockito with them all the time, accessed from the methods that call the static helper method.

There is a slight difference in how the compiler treats a static helper method. The byte code created will result in an invokestatic instruction, and if you remove the static the result will be one of the other instructions, like invokespecial. The difference there being that invokestatic loads the class to access the method, where invokespecial pops the object off the stack first. So there might be a slight performance advantage (maybe not).

Answer 4

That you can't mock them easily when you need to pretty much answers your own question.

Particularly when it's something as shown: making an HTTP call is expensive, and doing that for unit testing your code makes no sense–save that for integration testing.

Unit tests require known responses (and response codes) from HTTP calls, something that you can't do if you're calling someone else's service, using a network you don't control.

Answer 5

This explanation seems to me very straight forward, and easy to understand.

Declare utility methods as static, often makes your code easier to understand, which is a good thing.

But, there is a serious limitation to this approach though: such methods / classes can't be easily mocked.

So if a helper method has any external dependency (e.g. a DB) which makes it - thus its callers - hard to unit test, it is better to declare it non-static.

This allows dependency injection, thus making the method's callers easier to unit test.

Source: https://softwareengineering.stackexchange.com/a/111940/204271