Java – Converting static utility class into singleton

javalegacymockingsingletonunit testing

In company where I work we have lots of "utility" classes, each has lots of code inside (thousands of lines), and they are all static. And one static methods call anothers. The problem here is that when you want to test other classes you have lots of pain when tested function uses these static utility classes inside because you can not override them(as they are static). We are using mockito for testing. Powermock is not an option because it breaks bytecode sometimes and it's much slower than mockito.

What we decided – is to move all them to singletons, step by step, so when testing we can mock singleton instance and test method we want (or even test some utility method because they also need to be covered by autotests). It's simple to do that for small classes, but in our case there are 6006 lines class, 1261 lines class, 2231 lines class and so on. Converting them all into singletons manually is a lot of pain, so is there any way to do that automatically? Or maybe some helpers at least.

Example of untestable code:

static Object methodA()
{
      *very heavy code here*
}

static void methodB()
{
      *some code*
      xx = methodA();
      *here is code I've changed and want to test*
      *some code*
}

Let's imagine I changed something in methodB and want to test my change. So I don't want this methodA to be called as it's not related to the test. But I can't change what methodA does with mockito in such case. However it is possible if inside methodA there is a call to the same singleton method that can be mocked because it is not static.

So if you convert it to something like this

public class A {
    private A(){}

    public Object methodAInner()
    {
        *heavy code here*
    }
    public void methodBInner()
    {
        *some code*
        xx = methodA();
        *here is code I've changed and want to test*
            *some code*
    }


    private static A getInstance()
    {
        return SpringUtils.getBean(A.class); // here can be anything that supports testing
    }

    static Object methodA()
    {
        return getInstance().methodAInner();
    }

    static void methodB()
    {
        getInstance().methodBInner();
    }
}

You can make these SpringUtils return mocked object with real methodB and make almost no changes to existing code(useful for git blame), just remove static modifier and chnge methods names. An what is the most important for legacy – it allows not to change code that uses this utility class.

Best Answer

I'm all for fighting fire with fire, but turning bad code (lots of static methods) into other bad code (lots of singletons) doesn't sound like such a good idea to spend time on really.

From what you have written, this is all in a legacy code context, but you do have change access to the source where those static methods reside (maybe all, but the rest is too large to touch upon). So let's take a look at where you want to go ultimately:

  • Why do you want to do that work at all? because the current code is untestable and makes new code hard to test as well
  • Why is it untestable? because of all the static methods
  • Why do we have those static methods? because they are used everywhere, have recursive calls (rely on each other), and hence are hard to get rid off
  • Why do they all rely on each other? because utility classes have a tendency to do a lot of things instead of a single responsibility

My next question and suggestion to you would then be: Why don't you extract one responsibility after the other?

You may have a tough time identifying a responsibility, but you're probably having a concrete use case already that made you aware of this problem in the first place, so that should be a good start.

Once you have a separate, single responsibility and testable class available, you can still use it within the old class to replace parts of that class. Maybe you still have some duplication, because you needed that other static method's code in your new class, but it's still called from some other? Then it's a pointer to your next work area - identify what the common thing is that the two places share and extract it out from both.

Due to the sheer size you portray, this is going to be a slow and long running process, so every small responsibility that's out of that class has to be considered a win. Let the old static method rely on the new class, deprecate it, eradicate it whenever you see a call site. These things take time until you get to a point, where you can finally make that last investment and hole your team up for a day or two to get rid of all the remaining old and problematic code. As long as you take care, that no one uses these old methods for any new or modified code, you're getting closer to that goal. I wish you good luck for the journey.

Addendum - based on the additional information that these utility classes differ between different branches and different teams work with each of these: In short, you are trying to shoot a running target. Given the size of your codebase, the task itself is hard enough for a stable target, but I cannot recommend you continue down that path, until you first stopped that movement. Talk with the other teams and get them on board. If they object in any way shape or form, an already hard task becomes pretty much impossible. Hopefully, they all share your pain (or at least not caring about that part of the codebase) and you can get back to a unified state of these classes by merging them all.

Yes, I do suggest to merge all these branches w.r.t. the utility classes. I know it is its own hell. You have but three choices: make it a common problem (and share the solution work!), completely sever yourself, or just let it be as is.

The severing means that your team on your branch flat out defines itself as a different product line. You stop merging with the other guys and roll your own instead. It may not work, and even if it does, I have seen this come back to hit everyone in the company a few years down the road.

Anyways, every problem that cannot be solved locally within your own team('s codebase) is unfortunately immediately a matter of company politics. It's simple to pick a fight with your teammates to get rid of a nasty class in your team's code, but it's worth a second thought on whether you really want to go to war involving the other teams/stakeholders/managers/etc.

Related Topic