I have a large class (about 1500 LOC) and it uses different "strategies" to transform data from one object to another. I have here a representation of that class:
public class FooService implements FooProcessing {
FooRequestTransformer fooRequestTransformer;
AnotherService anotherService;
InstanceVar1 iVar1;
InstanceVar2 iVar2;
...
There is an interface (external to the class) that this class uses:
interface TransformerStrategy {
public FooRequest transform(FooResponse response);
}
Which is passed into this method (inside the FooService class):
private FooResponse getResponse(FooResponse fooResponse, TransformerStrategy transformerStrategy) {
FooRequest fooRequest = transformerStrategy.transform();
fooResponse = anotherService.bar(fooRequest);
return fooResponse;
}
The entry point is here which uses the getResponse()
method and creates a TransformerStrategy
anonymously:
public List<Foo> getFooForSomeFlow1(Param1 param1, Param2 param2, ...){
FooResponse fooResponse = anotherService.baz(fooRequest);
TransformerStrategy myTransformerStrategy = new TransformerStrategy() {
public FooRequest transform(FooResponse fooResponse) {
fooRequestTransformer.transform(param1, param2, iVar1, iVar2)
}
}
FooResponse fooResponse = getResponse(fooResponse, myTransformerStrategy);
...//other code
}
Now the problem is: there are several methods like getFooForSomeFlow1()
(inside FooService
) that all have their own anonymous implementation of TransformerStrategy
and subsequently call getResponse()
. As you can imagine, this is very messy, as well as confusing when you debug (i.e. you are stepping into getResponse()
and then all of a sudden you are back in getFooForSomeFlow1()
)
One possible solution (that comes to mind) is to organize these different strategies into a "Provider" class that will group them together somehow. Strangely enough, this class already includes this type of Provider class:
protected class StrategyProvider {
public ABCTransformerStrategy newABCTransformerStrategy(FooRequestTransformer transformer, Param1 param1, Param2 param2) {
return new ABCTransformerStrategy(transformer, param1, param2);
}
}
protected class ABCTransformerStategy implements TransformerStrategy {
protected FooRequestTransformer transformer;
protected Param1 param1;
protected Param2 param2;
//...constructor here
//...overridden transform method as above
}
In one of the comments it says, "Converted anonymous class to inner class for testing purposes". However, they only converted one of them, and left the rest of them. So it's like they started the process of refactoring and stopped in the middle.
So, I was thinking I could finish the process of refactoring and move all the anonymous classes to inner classes, and then finally move out these classes and StrategyProvider
into external classes.
The problem is that "converting anonymous to inner" adds more boilerplate (see ABCTransformerStrategy
above; I have to pass in all the data to the constructor) and I'm not really sure how much I'm gaining by doing this refactoring process.
I have two questions:
Thanks
I tried irreputable's approach and tried "Inline Method" on the getResponse()
method, but it was inlining some duplicate code (which is why we extracted that duplicate code into getResponse()
in the first place). I should have clarified that in my question that there is more code in getResponse()
than what I showed.
As for making a Factory, I couldn't justify introducing more boilerplate and LOC into the class, especially since I have to pass a lot of data into the Factory Method and/or Inner Class.
What we did instead was, we wrapped the the anonymous inner classes with a method like so:
private TransformerStrategy createTransformerStrategyWithABCValues(Param1 param1, Param2 param2, IVar ivar, IVar1 ivar2) {
return new TransformerStrategy() {
public FooRequest transform(FooResponse response) {
return FooRequestTransformer.transform(
param1,
param2,
iVar1,
iVar2);
}
};
}
Now the calling method looks like this:
public List<Foo> getFooForSomeFlow1(Param1 param1, Param2 param2, ...){
FooResponse fooResponse = anotherService.baz(fooRequest);
TransformerStrategy myTransformerStrategy = createTransformerStrategyWithABCValues(param2, param2, iVar1, iVar2);
FooResponse fooResponse = getResponse(fooResponse, myTransformerStrategy);
...//other code
}
In the process of doing this, we found that the 5 strategies had some duplication, so we were able to get that down to 3 strategies. We got rid of the StrategyProvider class as well as the inner class that was provided.
With this approach it's now somewhat easier to debug/maintain, and we were able to cut quite a bit of duplication out of the code.