2

I'm trying to refactor some code, and one of the major changes is to remove the (ab)use of static classes to give global state.

I've tried to split up some of the 'global state' functionality with POJOs that can be passed around.

But the further down the rabbit hole I go the more and more I feel like this is an Anti-Pattern.

Is holding state in POJOs and then passing them around bad form? It means the outline of my methods often look like this:

public MyPojo makeReport(int myInt, String pathToInput, MyPojo myPojo){
    ...
    if(myPojo.whatKindOfReport()==Reports.reportTypes){
        ...
    }
    ...
    myPojo.isWritingReportToFileSuccessful(true);
    ....
    return myPojo;
}

Where the POJO 'state' is set from the GUI that the user interacts with. For instance with tickboxes for report types etc.

But this blocks other return types, and seems overly complicated.

I've taken this approach because the application has several 'paths' (see whatAmIDoing) the user can take in running it, each of which has several stages. What options the user has depends heavily on what path they pick and then from there, the result of that choice (normally, success or failure, see myPojo.isSomethingSuccessful(true)). I'm trying to encode that information in a POJO rather than a Global State Class.

Is this an anti-pattern? How can I get round this?

Edit: I think what I'm after is something like Chain of Responsibility. After looking at this answer I'm not sure.

Community
  • 1
AncientSwordRage
  • 1,043

2 Answers2

2

This looks as if the doSomething method should be member of MyPojo class. Getting object as a parameter and then returning it? Definitely doesn't look good.

The way you describe your code seems to be even more indicative of some kind of class structure. Using enum as descriptor to what is done is clear smell that should be replaced with class hierarchy where each class represents one path the code can take. As described here.

And as Idan said, making the object a field of class that contains the doSomething method instead of parameter to the method itself can be better. Also, you can make the object as constructor parameter, so you can use dependency injection.

Euphoric
  • 38,149
1

Stop thinking in terms of operations and start thinking in terms of data.

doSomething is not a static method, therefore it belongs to an object which can hold state - lets call the class of that object Foo, and say you have created an instance named foo.

Now, instead of saying that foo.doSomething needs an instance of MyPojo, you should say that foo needs to do something with MyPojo. So - you can do foo.setMyPojo(myPojo); and then you can call foo.doSomething without the MyPojo argument - because it already has it. In addition, when you call other methods of foo it already has myPojo set! And the best thing - this allows you to draw a graph of dependencies based on objects, not methods.

Idan Arye
  • 12,145