Do it short but do it right !
For my very first article on JCG, I wanted to show you how you can easily improve the readability of your code, exploiting the power of the Java language, even for pretty basic things.
Let’s start with a concrete example:
String color = "green"; ... if ( color!=null && color.equals("red") ) { System.out.println("Sorry, red is forbidden !"); }
One of the first lessons you probably learned from your Java (or Object-Oriented programming) teacher is the importance of testing the nullity of an object before invoking a method on it. Null pointer exceptions (NPEs) are indeed among the most common (and irritating) faults raised in the code of object-oriented languages.
In the above example, it is safe to ensure the ‘color’ String object is not null before comparing it to a constant. I personally have always considered this as an unnecessary burden on the programmer – especially for modern OO languages such as Java. As a workaround, there exists a (really stupid) trick for rewriting the condition without having to test for nullity. Remember, the equals() method is symmetric (if a=b then b=a).
if ( "red".equals(color) ) { System.out.println("Sorry, red is forbidden !"); }
At first glance, it might be seen a bit contra-natural when read, but eliminating the polluting code is certainly not worthless.
Let’s continue our example, and imagine we now want to compare our color with multiple values. Java beginners would usually write something like:
if ( "red".equals(color) || "yellow".equals(color) || "blue".equals(color) ) { System.out.println("This is a primary color"); }
I have sometimes met more experienced Java programmers shortening such long if statement with:
if ( "red|yellow|blue".indexOf(color)>=0 ) { System.out.println("This is a primary color"); }
Smart isn’t it ? Not that much actually. Playing with substrings can be a dangerous game. For instance the following code might not give the expected results, especially if you are a man:
String type = "man"; ... if ( "woman|child".indexOf(type)>=0 ) { System.out.println("Women and children first !"); }
If you are looking for a good balance between elegance and readability, you had better opt for one of the following alternatives.
import java.util.HashSet; import java.util.Set; public static final Set<string> PRIMARY_COLORS; static { PRIMARY_COLORS = new HashSet<string>(); PRIMARY_COLORS.add("red"); PRIMARY_COLORS.add("yellow"); PRIMARY_COLORS.add("blue"); } ... if ( PRIMARY_COLORS.contains(color) ) { System.out.println("This is a primary color"); }
Few people know it (I admit the syntax is a bit strange), but there is still a way to reduce code verbosity when initializing the Set of primary colors:
public static final Set<string> PRIMARY_COLORS = new HashSet<string>() {{ add("red"); add("yellow"); add("blue"); }};
In the event concision of code becomes an obsession, the Java Collections Framework can also come to the rescue:
import java.util.Arrays; import java.util.Collection; public static final Collection<string> PRIMARY_COLORS = Arrays.asList("red", "yellow", "blue"); ... if ( PRIMARY_COLORS.contains(color) ) { System.out.println("This is a primary color"); }
The final keyword prevents the PRIMARY_COLORS variable from being re-assigned to another collection of values – this is particularly important when your variable is defined as public. If security is a major concern, you should also wrap the original collection into an unmodifiable collection. This will guarantee a read-only access.
import java.util.Arrays; import java.util.Collection; import java.util.Collections; public static final Collection PRIMARY_COLORS = Collections.unmodifiableCollection( Arrays.asList("red", "yellow", "blue") );
It must be noticed that, though more readable, using a collection of values (especially with large collections) will generally remain slower than (I would rather say: not as faster as) classical lazy OR’s (ie using ‘||’ instead of ‘|’) because of the short-circuit evaluation. I tend to think that nowadays such considerations become futile.
After 16 years of complaints, Java 7 has – at last! – introduced the support of String in switch-case statements. This allows us to code things such as:
boolean primary; switch(color) { case "red": primary=true; break; case "green": primary=true; break; case "blue": primary=true; break; default: primary=false; } if (primary) System.out.println("This is a primary color");
Let us finally end with what is probably the most object-oriented solution to our (so to say) problem. Java enumerations are primarily classes, and can therefore have methods and fields just like any other classes. By applying the Template Method design pattern, one can define an abstract method (modeling the test) which has to be implemented by all subclasses (modeling the response of the test applied to a particular item of the enumeration):
Color c = Color.valueOf("RED"); if ( c.isPrimaryColor() ) { System.out.println("This is a primary color"); } public enum Color { RED() { @Override public boolean isPrimaryColor() { return true; } }, BLUE() { @Override public boolean isPrimaryColor() { return true; } }, YELLOW() { @Override public boolean isPrimaryColor() { return true; } }; GREEN() { @Override public boolean isPrimaryColor() { return false; } }; public abstract boolean isPrimaryColor(); }
The resulting code is clear and self-documenting. Using this pattern is a great alternative in many cases to the more common “if – else if” logic since it is easier to read, extend, and maintain.
To conclude, I would say that, as very often – and this is the power of the Java language – for one problem, there exist so many different solutions in terms of implementation. But deciding which one is the best is another story…
Reference: Do it short but do it right ! from our W4G partner Bernard Ligny.
Related Articles :
As an alternate which seems to me to be even clearer in regard to intent, as well as using a single instance of a function rather than multiple, I offer the following:
Color c = Color.valueOf(“RED”);
if (c.isPrimary()) {
System.out.println(“This is a primary color”);
}
public enum Color {
RED(true),
BLUE(true),
YELLOW(true),
GREEN(false);
private boolean primary;
Color (boolean primary) {
this.primary = primary;
}
public boolean isPrimary() {
return primary;
}
}