Avoding multiple ifs

[Amit A. Patil]

I have a function to which a String is passed
rawStr="BallRed0550"
which mean ball is red of quantity 5 and weight 50 grams.
i do certain processing based on this string,
if("Blue".equals(rawStr.subString(0,4))){

do something...
}else("Red".equals(rawStr.subString(0,4))){
do something
}else {
do something...
}
Someone suggested using a hasMap will be faster..i mean hashmap of objects with keys as "Blue" ,"Red" and calling method of that object .Is it true?
What is the fasted way of doing this?

[Anupam Sinha]

I would suggest having constants to define the colour and then if weights are predefined use constants for them as well. Parsing a String doesn't sound right. Secondly how do you determine if the colour of the ball is RED or BLUE with substring?

[Amit A. Patil]

I have no control this string is sent by a leagacy system cant change it

[Peter Chase]

OK, so you can't change the String coming from the legacy system, but you can parse it just once and convert it to something that can be better used in the rest of your program. Repeatedly substringing and doing string comparisons is ugly and inefficient.

I would suggest a static hash map with keys that are the colours, and values that are objects of an appropriate type. Perhaps define an enum type for the colours.

If you use a hash map, use consistent case (e.g. lower case) for the keys, then convert all incoming strings to that case. That's assuming that the case in the incoming strings is not significant, of course.

[Stan James]

Re the HashMap idea: refactoring a switch case or if-else into a Strategy is a neat option.

String color = rawStr.subString(0,4);
Strategy strategy = getStrategyFor( color );
strategy.execute( args );

What's really neat is that you can close this code for future modification and add new colors and strategies through the creator method, getStrategyFor(). If I expected lots of change I might move that to a factory-like class and set it up from configuration so I could add new colors and behaviors without touching this class ever again.

[Amit A. Patil]

I did a quick check HashMap is always faster
package test.server;

import java.util.HashMap;

public class TestIfs {

/**
* @param args
*/
public static void main(String[] args) {
// TODO Auto-generated method stub
int count=0;
for(int i=0;i<10000;i++){
if(multipleifs()> multipleMap()){
count++;
}
}
System.out.println("Ifs were slower "+count +"out of 10000");

}

public static long multipleifs() {
String s="Red";long avg=0;
for(int i=0;i<10000;i++){
long prev=System.nanoTime();
if("Black".equals(s)){

}else if("Green".equals(s)){

}else if("Grey".equals(s)){

}else if("White".equals(s)){

}else if("Red".equals(s)){

}
avg+=System.nanoTime()-prev;
}
// System.out.println(avg/10000.0);
return avg;
}

public static long multipleMap(){
HashMap map=new HashMap();long avg=0;
map.put("Red","Red");
map.put("Black","Black");
map.put("Green","Green");
map.put("White","White");
map.put("Grey","Grey");
for(int i=0;i<10000;i++){
long prev=System.nanoTime();
map.get("Green");
avg+=System.nanoTime()-prev;
}
// System.out.println(avg/10000.0);
return avg;
}

}

[Stan James]

I'm a little surprised at the result, but then again you loaded the dice by always looking for "red" which is at the end of the if-else chain. The longer the chain gets the more effect something like that might have.

I almost always make choices like this based on what improves the design - hence my emphasis on Bertand Meyers' "open closed" aspect of the map loaded from configuration - and almost never on performance.

Then again, I rewrote (not refactored) a complex utility class this year and made it several times faster more or less by accident. Sometimes better is just better.

[Jim Yingst]

One problem with your initial code is how to extract the "Red" or "Blue" from the initial string. such as "BallRed0550". Note that rawStr.subString(0,4) extracts the first four characters of rawStr - which is the word "Ball", typically. You could use substring(4, 7) to get "Red" - but if it's "BallBlue0123" then you need substring(4, 8). The HashMap approach will only work if you can extract the exact string in advance. Perhaps you can do this with regular expressions:

private static final Pattern ballColorPattern = Pattern.compile("Ball(\\p(Alpha}+)\\d+");
 
...
 
Matcher m = ballColorPattern.matcher(rawStr);
if (m.matches()) {
    String color = m.group(1);
    // look up color in HashMap
}

This assumes that the color is always preceded by "Ball" and followed by a number. If that's not the case, you will need to know more about what types of rawStr are possible here.

Note that one advantage of the original chain of ifs is that you have much more flexibility in what you test on each line. You don't have to use .equals() only. For example you could say

if (rawStr.startsWith("BallRed")) {
    ...
} else if (rawStr.startsWith("BallBlue")) {
   ...
}
...

Now I do generally recommend the HashMap approach for this sort of thing whenever possible. But sometimes it just doesn't work, and a chain of if statements is what you need.