Avoiding if/else constructs

[David Follow]

Hi all,
say I have a web application (e.g. made with Struts) where
I have to respond on a certain item. Say "color".
The problem is that I have to cover all the colors.
Eventually I will end up having a huge list of nested if/else statements, e.g.
if(color.equals("red"))
forward = ....;
else if(color.equals("blue"))
forward = ....;
else if(color.equals("green"))
forward = ....;
else if(color.equals("white"))
forward = ....;
else if(color.equals("black"))
forward = ....;
else if(color.equals("yellow"))
forward = ....;
//....and so forth.
Is there a better way of doing that?
Can I use a pattern (e.g. command pattern) that does a better job?
What about polymophism? The thing is that I don't want to make it
too complicated. All I want to do is get rid of the huge if/else structure.
Any ideas?
TIA, Misos

[Joe Ess]

Why don't you declare forwards (action? My Struts is rusty) for each of the colors (or some name based on the color)? That way you just call forward(someName+colorName) or something like that. Of course, should someone somehow submit a name that isn't covered in your Struts config file. . .

[rom chatterjee]

Dont know if this too hacky, but you could extend Color adding static IDs, eg MYColor.RED_ID=1, MYColor.BLUE_ID=2, and then switch on the ID.

[Stan James]

If the values you are assigning are singletons or Strings or something simple, you could cache a bunch of them in a HashMap keyed by the color name.

forward = (WhateverAForwardIs)myMap.get(color);

If you have to create a new thing every time, you might cache class names.

className = (String)myMap.get(color);
if (null == className) throw some Exception;
forward = (WhateverAForwardIs)Class.forName(className).newInstance();

Either way, you might have an initialization routine that loads up the map from configuration data, so you can add new things without opening up the code. Hope that helps!
[ October 30, 2003: Message edited by: Stan James ]

[Jim Yingst]

Dont know if this too hacky, but you could extend Color adding static IDs, eg MYColor.RED_ID=1, MYColor.BLUE_ID=2, and then switch on the ID.
If you have a class for Color, then you also might just make this forward thing an instance variable in the class. Then no need for a switch at all. But I have no idea if that would make sense for your design. And I don't know much about struts; Joe's advice may well be more applicable there. But one other possibility here is to put all the different forwards into a Map:

private Map forwardMap = new HashMap();
{
    forwardMap.put("red", "fwd1");
    forwardMap.put("blue", "fwd2");
    forwardMap.put("green", "fwd3");
    forwardMap.put("white", "fwd4");
    forwardMap.put("black", "fwd5");
    forwardMap.put("yellow", "fwd6");
}
private String getForward(String color) {
    return (String) forwardMap.get(color);
}

This is well-suited to a situation where you have a large number of options, as the get() method is a constant-time operation. You can also easily load the forewardMap from a file of some sort, for greater flexibility. Of course struts may already have somethign like this somewhere for all I know...

[David Follow]

Hi all,
thanks for all the good ideas.
One more thing, it doesn't really have to be Struts, it's more or less a general thing where I would like to avoid large if/else structures.
To make things even more complicated my if/else can look something like this:

 
if (color.equals("red"))
            actionForward = someMethod( mapping, paf, request, response );
else if (color.equals("green") || color.equals("blue") )
            actionForward = someOtherMethod(mapping, paf, request, response);
else if (color.equals("black") && color.equals("white"))
            actionForward = differentMethod(mapping, paf, request, response);
//...and so forth

Any ideas on this one?
How could I solve this with a HaspMap?
TIA, Misos

[Stan James]

Now you're getting to where polymorphism comes in handy. The Command pattern talks about encapsulating an action in an object. So let's make a different class for each different action and initialize a set of them in a factory class:

myMap.put("red", new RedCommand());
myMap.put("green", new GreenCommand());
myMap.put("blue", new BlueCommand());

Now when somebody gives you a color:

Command command = CommandFactory.getCommandForColor(color);
command.execute(args);

That doesn't really handle the && and || stuff you showed. BTW: The && you showed will never be true. Right?
This principle is alive in the "front controller" pattern like Struts. Given an inbound GET or POST request, get the right servlet or command from a factory and execute it. There the configuration is in XML instead of the kind of hard-coded factory initialization I showed. The whole business of decoupling and avoiding giant case statements can be a lot of fun!

[Jim Yingst]

That doesn't really handle the && and || stuff you showed. BTW: The && you showed will never be true. Right?
Ignoring the &&, the || can easily be handled with something like

Command rCmd = new RedCommand();
Command bgCmd = new BlueGreenCommand();
myMap.put("red", rCmd);
myMap.put("green", bgCmd);
myMap.put("blue", bgCmd);

[David Follow]

Originally posted by Stan James:
Now you're getting to where polymorphism comes in handy.

Hi, this may be true, but in my if/else struct all I have to do is add
a line or two to do my thing. With the polymorphism approach I will have
to write a separate class of every potentially "little thing" I want to do.
Now which one will be more readable? The design of the polymorphism approach
may be nicer but from a point of readability (and eventually lines of code)
the complex if/else struture is in my opinion more obvious.
What do you think?

[Loren Rosen]

If you really have just one place where the if/else occurs that it's certainly arguable that using polymorphism is over-engineering. But what usually happens is that the same if/else ends up in dozens of places, and the result is a lot harder to maintain that a set of subclasses.

[Stan James]

Yes, adding classes and configuration seems like (ok, it IS) overkill for simple situations, like a one-man project. But note you said you have to open up the code and add a line. Every time you open it, your manager might insist you regression test the whole system to make sure you didn't break it. And you know that guy at the of the aisle will break it every time he opens it. Ouch! Or maybe you'd like to sell your class in compiled form. I could buy it and add more color-action combos in configuration file without ever seeing the source. Cool, no?
You do have to learn to see the line between beautiful theory and getting the job done. This is not easy, because it moves on every assignment. XP has a precept to Do The Simplest Thing That Can Possibly Work. Start there. Refactor goodness in when the risk of that guy down the aisle starts to bother you.

[Jim Yingst]

This also depends on how many different types of behavior you really have here. If you have 100 different color names that need to be mapped to 100 different method calls, then writing 100 different classes for this to take advantage of poymorphism is proabably overkill. Then again, writing 100 different method calls isn't too great either; I'd be looking hard to see if there isn't some better way find similarities in the behavior of the different method calls, which might suggest some refactoring which is more readable. Maybe some of those method calls are identical except they use different numbers inside? Then make the numbers parameters which can be set by a constructor, so maybe you can have something like

myMap.put("red", new ColorCmd(5));
myMap.put("green", new ColorCmd(17));
myMap.put("blue", new ColorCmd(32));

(I'm just making up numbers here; no idea what they might mean.)
And if you have 100 different colors that map to, say, 5 different behaviors, then writing 5 short classes (plus one interface) and putting 100 entries in a Map is probably a pretty good way to go.
Polymorphism may not be the end-all solution here, but I think it's at least worth considering.