I have a refactoring situation that I cannot find an elegant solution for...
Disclaimer: Keep in mind that I am oversimplifying this example to reduce clutter, and not disclose things I am not allowed to disclose :) As such, please do not assume that this is the ONLY code in my giant codebase, and offer solutions that cut corners or change parts of the design that I mention cannot be changed due to outside constraints.
The facts:
I have a utility class, it has a bunch of static methods, they utilize a singleton resource:
public final class Utility
{
private static final Resource RES = Resource.getInstance();
private Utility() {} // Prevent instantiating Utility
public static boolean utilMethodOne() { return RES.isSomething(); }
public static int utilMethodTwo() { RES.getNumThings(); }
...
public static void utilMethodInfinity() { ... }
}
Utility is in a library JAR that is used by several applications in a large codebase -- let's say on the order of 10,000 calls to its static methods, e.g.: if(Utility.utilMethodOne()) { ... }
Resource is an outside class from another library JAR.
Resource also has a method Resource.getInstance(String name) that will return a named instance, which may relate to a different underlying resource based on the name (internally it keeps the named resources in a Map<String,Resource>).
Resource.getInstance() returns the equivalent of Resoruce.getInstance(""), aka the default instance.
The situation:
The Utility needs to be enhanced to now execute against one of several resources, so my plan is to make the Utility an instantiable class with a non-static Resource member variable. Something like this:
public final class Utility
{
private Resource res;
public Utility(String resName)
{
this.res = = Resource.getInstance(resName);
}
public boolean utilMethodOne() { return this.res.isSomething(); }
public int utilMethodTwo() { this.res.getNumThings(); }
...
public void utilMethodInfinity() { ... }
}
Now all this is great, and I can start creating Utility objects that access their specified resource instead of just the default one. However, as I mentioned, there are 10-100K method calls that are now invalid as they were calling static methods!
The problem:
My plan was to keep the static methods in Utility, and have them use the default instance from Resource, while adding in non-static variants for the instantiated Utility objects that use their "local" resource reference.
// Best of both worlds:
public static boolean utilMethodOne() { return RES.isSomething(); }
public boolean utilMethodOne() { return this.res.isSomething(); }
Maybe I can't have my cake & eat it too:
error: method utilMethodOne() is already defined in class Utility
public static boolean utilMethodOne(String sql)
So it seems I am going to have to either...
- Introduce a whole new BetterUtility class for places that want to use the named-resources.
- Update 10,000 places to instantiate & use the revised Utility object.
- ...? (hint: this is where your suggestions come in!)
I really don't like 1 or 2 for a variety of reasons, so I need to ensure there is no better 3 option before settling. Is there any way to retain a single class that can provide both the static & non-static interfaces in this case?
UPDATE 2020-06-01:
I am coming to the realization that this magical option 3 doesn't exist. So out of my original two options I think #2 is best as it's just one time "just get it out of the way and be done with it" type effort. Also incorporated some of your suggestions in the design(s).
So now that I have a direction on this, I am left with [hopefully only] one more key decision...
- Update all the calls to create new objects
// For a one-off call, do it inline
boolean foo = new Utility("res1").utilMethodOne();
// Or when used multiple times, re-use the object
Utility util = new Utility("res1");
boolean foo = util.utilMethodOne();
int bar = util.utilMethodTwo();
...
Given the amount/frequency of usage, this seems like a whole lot of wasted efforts creating short-lived objects.
- Follow the pattern that Resource itself uses, creating my own named-singleton map of Utilities (1:1 with their respectively named Resource)
public final class Utility
{
private static final Map<String,Utility> NAMED_INSTANCES = new HashMap<>();
private Resource res;
private Utility(String resName)
{
this.res = Resource.getInstance(resName);
}
public static Utility getInstance(String resName)
{
synchronized(NAMED_INSTANCES)
{
Utility instance = NAMED_INSTANCES.get(resName);
if(instance == null)
{
instance = new Utility(resName);
NAMED_INSTANCES.put(resName, instance);
}
return instance;
}
}
public boolean utilMethodOne() { return this.res.isSomething(); }
public int utilMethodTwo() { this.res.getNumThings(); }
...
public void utilMethodInfinity() { ... }
}
// Now the calls can use
Utility.getInstance("res1")
// In place of
new Utility("res1")
So essentially this boils down to object creation vs. a synchronization + map lookup at each usage. Probably a little bit of premature optimization here, but I'll probably have to stick with this decision long term.
UPDATE 2020-06-29:
Didn't want to leave an "Internet dead end" here... I did eventually get all the call sites updated as described above (including option #2 from the 2020-06-01 update). It has made it through all testing and been running in production for a week or so now in various applications.