How to refactor large class that uses "Strategies"?

Question

Problem

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())

Possible solution

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:

1. Should I continue with this approach?
2. Or is there another design pattern that I can apply that would be more appropriate and really simplify this code?

Thanks

Answer 1

Based on the code sample you provided, it is superficially complicated. Why not simply

public List<Foo> getFooForSomeFlow1(Param1 param1, Param2 param2, ...)
{
    FooResponse fooResponse = anotherService.baz(fooRequest);

    FooRequest  fooRequest = fooRequestTransformer
                                 .transform(param1, param2, iVar1, iVar2); 

    FooResponse fooResponse2 = anotherService.bar(fooRequest);
    ...//other code
}

Unless there's something else going on that you haven't shown us.

Answer 2

Your provider class is actually a Factory pattern, which is exactly what I was going to suggest.

By moving the logic out of the getFooForSomeFlow1 and similar methods, you've created very loosely coupled code, which is always desirable.

Simply out of personal preference, I would have one method, used to return instances, that takes an int value as the Discriminator. These int's could be static final variables inside the provider class, like:

public static final int ABCTransformerStrategy = 1;

Additionally, by making the Factory class Abstract, you will remove the need to instantiate the class every time you need to use it, making for much cleaner code.

Edit

As suggested, using Enum is a much more desirable, and semantically correct way of translating values to the concrete classes they represent.