aspose file tools*
The moose likes OO, Patterns, UML and Refactoring and the fly likes howto avoid this complexity (state pattern?) Big Moose Saloon
  Search | Java FAQ | Recent Topics | Flagged Topics | Hot Topics | Zero Replies
Register / Login


Win a copy of Spring in Action this week in the Spring forum!
JavaRanch » Java Forums » Engineering » OO, Patterns, UML and Refactoring
Bookmark "howto avoid this complexity (state pattern?)" Watch "howto avoid this complexity (state pattern?)" New topic
Author

howto avoid this complexity (state pattern?)

Peter Primrose
Ranch Hand

Joined: Sep 10, 2004
Posts: 755
Hi experts,

I have a class 'Debit' that has 3 fields:
1. status (1,2,3,4,5)
2. type (1,2,3)
3. district (1,2)

I have another class that determine what 'action' should be taken depends on the Debit class.

Example:
if (status==1 && type==4 && district==2) do A
if (status==5 && type==2 && district==1) do B

As you can see there could be 5*3*2=30 potabilities. So i did this:
I created an interface that has the following:

DebitAction:

boolean isValid();
void execute();

and each class that implements this interface is executing the isValid method (has 1 if statement); and if the return is True - it invokes the method execute. So instead of having 30 if statements - I have 30 classes in a while loop that executes all 30 classes (this is easier to maintain but still a pain).

Question: Am I on the right track or there could be a better solution.


thank you!
Frank Carver
Sheriff

Joined: Jan 07, 1999
Posts: 6920
I have a class 'Debit' that has 3 fields:
1. status (1,2,3,4,5)
2. type (1,2,3)
3. district (1,2)


Fair enough, although describing a class in terms of the data it holds rather than in terms of its behaviour is often a warning sign of an "anaemic" design.

I have another class that determine what 'action' should be taken depends on the Debit class.
Example:
if (status==1 && type==4 && district==2) do A
if (status==5 && type==2 && district==1) do B


Is there any particular reason why you have chosen to put this processing in a different class? My immediate instinct is that such decision making (which depends intimately on the internal state of an object) should probably be part of the same class.

So instead of having 30 if statements - I have 30 classes in a while loop that executes all 30 classes (this is easier to maintain but still a pain).

Firstly, do you really need all 30 possibilities? In most systems like this I have encountered, some of the possible combinations are either invalid, or very similar to others. Usually the range of behaviour is considerably narrower than the domain of possible input combinations.

Even if all 30 cases have distinct behaviour, I still feel that looping through all the cases and attempting to match is a very inefficient way to approach the problem.

You don't say explicitly, but I'm guessing that each Debit object probably contains the same values throughout its lifetime (as a "data holder" it probably originates from a user-input form or from some external storage such as a database, and is simply disposed of when no longer needed)

If this assumption holds, then the decision of which "execute" behaviour to associate with the Debit can be done once when the Debit object is created; perhaps by using a Strategy Pattern:



In this model, to "execute" a particular debit, just call its "execute" method; no looping and comparing necessary.

This does require, of course, that the correct Action is passed in at the time the Debit is created. A typical way to achieve this is to use a creation service such as an Abstract Factory or a Builder.



The only tricky bit is to ensure that the appropriate Action is associated with each Debit.

As a first step, I'd suggest re-using your existing validate-then-execute loop logic, but instead of calling "execute" on the matching action, pass it into the constructor of the new Debit.

Once this is working, there are no doubt a few minor improvements and optimisations, but if the creation of Debit objects is relatively rare compared to their use, then optimising may not even be worthwhile. Suggestions might include:
  • Arrange the actions in a Tree rather than just a List (deciding based on "district", for example, should immediately enable you to halve the number of remaining actions which need to be checked)
  • Clump the actions further based on similarity of behaviour, and match a range of values to a single Action which can do the behaviour for the whole group


  • Does any of that help?


    Read about me at frankcarver.me ~ Raspberry Alpha Omega ~ Frank's Punchbarrel Blog
    Stan James
    (instanceof Sidekick)
    Ranch Hand

    Joined: Jan 29, 2003
    Posts: 8791
    I'd likely approach "find the right action" with a map. You could key the map by status + type + district, eg "1.3.2" I'd probably think again if many keys mapped to the same action - it might be possible to reduce the selection logic.

    There would be many ways to hang this together ... just a few ...


    A good question is never answered. It is not a bolt to be tightened into place but a seed to be planted and to bear more seed toward the hope of greening the landscape of the idea. John Ciardi
    Peter Primrose
    Ranch Hand

    Joined: Sep 10, 2004
    Posts: 755
    Sheriff,

    First- thank you for your reply. Second, The Debit class has other fields such as: debitAmount, debitDate, issuingUser...
    Also, the 'state' of the debit is dynamic, meaning that it changes (if it has been paid - status=2, dispute=3, fullyPaid=4...)

    All cases are optional (may not occur) but I still have to address them all. I think I didn't explained the question properly, so maybe an example will have it:

    Say I have 10,000 debits and each has a different characteristic of status, type and district. Every week the end-user would like to go through them and 'do some action' regarding Status=2; Type=3 and District=1 (and then S=1; T=2, D=2 etc...)

    what I have in mind is something like this:



    the isVald - will check if the status, type, district are valid (with ref to usre settings) and if so will execute.

    I'm not sure how can you solve this without a loop.
    your thoughts, thanks again!
    Frank Carver
    Sheriff

    Joined: Jan 07, 1999
    Posts: 6920
    The Debit class has other fields such as: debitAmount, debitDate, issuingUser...

    No problem so far. They are just data, and have no significant impact on the algorithm side of things.

    Also, the 'state' of the debit is dynamic, meaning that it changes (if it has been paid - status=2, dispute=3, fullyPaid=4...)

    OK. It seems you have two choices here: One choice is to determine what to do with a debit based on its state at the time it is needed (along the lines of your loop example). Another choice is to treat all the fields of a Debit as immutable, and just create a new Debit based on the old fields and the new status if/when the status changes.

    My second option may sound strange, but it is exactly what Java does with the likes of Integer and String objects, so don't reject it out of hand.

    All cases are optional (may not occur) but I still have to address them all.

    I'm not sure this is entirely true. Logically, you only need to address the cases having Debits which match them. Any others will (by definition) never be needed, and repeatedly asking them if they are valid seems a big waste of time and effort.

    Say I have 10,000 debits and each has a different characteristic of status, type and district. Every week the end-user would like to go through them and 'do some action' regarding Status=2; Type=3 and District=1 (and then S=1; T=2, D=2 etc...)

    Perhaps I'm still not entirely understanding your problem.

    Are you saying that the user chooses a subset of Debits to act on (for example, he/she might choose to work only on Debits with Type=3 and District=1, or only on Debits with status 2 this week) but the action to be performed is the same for all matching Debits this week?

    Or are you saying that all Debits are processed every week according to the same rules, but that the action needed for each Debit depends on its S,T and D?

    I'm not sure how can you solve this without a loop.

    If your problem is more like my first description, above, then it seems that some sort of filter process which selects the appropriate set of Debit objects (are they stored in a database? can you use SQL for this?) then calls "execute" for each of them would be appropriate

    If your problem is more like my second description, above, then I think my suggestion from my previous post still seems suitable, especially if you take the approach of creating a new Debit "Value Object" (with, therefore, a new associated action) if/when the status changes,

    Or is your problem not much like either of my descriptions?
    Stan James
    (instanceof Sidekick)
    Ranch Hand

    Joined: Jan 29, 2003
    Posts: 8791
    Say I have 10,000 debits and each has a different characteristic of status, type and district. Every week the end-user would like to go through them and 'do some action' regarding Status=2; Type=3 and District=1 (and then S=1; T=2, D=2 etc...)

    Looping sounds like the deal here. You could try to filter your collection so it only contains the Debits we care about - perhaps by a database query? Or you could loop through all of them and take action only on the ones that match. And we can do that without an if-test:

    What about the ones that should do nothing? Well, doing nothing is a perfectly good Strategy. Configure the factory to return the ProperAction strategy for the chosen states and a DoNothing strategy for other states.

    BTW: I like the immutable option. Then the getStrategyFor() call could move to the constructor.
    [ November 17, 2007: Message edited by: Stan James ]
    Peer Reynders
    Bartender

    Joined: Aug 19, 2005
    Posts: 2922
        
        5
    Originally posted by Peter Primrose:
    Say I have 10,000 debits and each has a different characteristic of status, type and district. Every week the end-user would like to go through them and 'do some action' regarding Status=2; Type=3 and District=1 (and then S=1; T=2, D=2 etc...)


    For maximum flexibility and configurability you can always use the specification pattern (PDF) - though this application is somewhat trivial.



    Output:
    Peter Primrose
    Ranch Hand

    Joined: Sep 10, 2004
    Posts: 755
    thank you all Peer, Stan and Frank for your replies.
    your answers put some light on my problem and I revised my solution accordingly. Good for me I also learned about the spec-design-pattern
     
    I agree. Here's the link: http://aspose.com/file-tools
     
    subject: howto avoid this complexity (state pattern?)