A minor glitch in type-safe enums
I’m quite fond of the type-safe enum design pattern. However, Wolfgang Hoscheck recently identified a place in XOM where I had introduced a bug as a result of it. Here’s an example:
public class FontStyle {
private int value;
public static FontStyle PLAIN = new FontStyle(1);
public static FontStyle BOLD = new FontStyle(2);
public static FontStyle ITALIC = new FontStyle(3);
public static FontStyle BOLD_ITALIC = new FontStyle(4);
private FontStyle(int value) {
this.value = value;
}
}
The bug shows up in another class that reference the FontStyle
class:
private FontStyle style;
public void setStyle(FontStyle style) {
this.style = style;
}
Do you see the bug? The problem is that there is one thing that can be passed into setStyle
that is not one of the four legal values. Can you guess what it is?
null
This makes the pattern somewhat less convenient than I thought. It’s not hugely inconvenient, but if you’re using the type-safe enum object in multiple places, then you need to check for null in each of those places. For example,
public void setStyle(FontStyle style) {
if (style == null) {
throw new NullPointerException("Null style constant");
}
this.style = style;
}
Is there a way around this? We could certainly make null one of the legal values. However, that’s only a technical fix. Chances are if someone passes in null as a value then it’s a programmer error and should be signaled with an exception.
Do Java 5 enums fix this?I decided to find out. Here’s a simple Java 5 program that attempts to null an enum:
public class Test {
public enum Season { WINTER, SPRING, SUMMER, FALL };
public static void main(String[] args) throws Exception {
Test t = new Test();
t.setSeason(null);
System.out.println(t.season);
}
private Season season;
private void setSeason(Season season) {
this.season = season;
}
}
And here’s the output:
null
Looks like Java 5 enums do not fix the problem. When you think about it, it would be hard to be otherwise. If an enum type couldn’t have a null value then code like this would also be illegal:
Season s = null;
In fact, how would you handle an uninitialized variable or field of enum type? I suppose each enum could come with a default value, but that’s not how enums are defined.
I’m afraid there’s nothing to do but remember the null checks when using enums, type-safe, Java 5, or otherwise.
January 14th, 2006 at 7:42 pm
Just a wild guess, but using an aspect for all method signatures that have a formal parameter with a sub-type of Enum could help.
The aspect would check the parameter for null and throw a suitable expression.
January 17th, 2006 at 7:02 pm
Mark your [JDK 1.4] enumerations final also. As stated, the statement
FontStyle.PLAIN = FontStyle.BOLD;
is legal. Change that block to read
public static final FontStyle PLAIN = new FontStyle(1);
public static final FontStyle BOLD = new FontStyle(2);
public static final FontStyle ITALIC = new FontStyle(3);
public static final FontStyle BOLD_ITALIC = new FontStyle(4);
Or consider using the Jakarta Commons Lang Enum and ValuedEnum classes.
If you don’t use an aspect, as Jochen suggested, put checks in your set methods, constructors, and readResolve methods.
public void setFontStyle(final FontStyle style) {
if (style == null) {
throw new IllegalArgumentException(“Null style”);
}
this.style = style;
}
If no object can ever have a null font style, you don’t need to check.
January 18th, 2006 at 2:04 pm
The Nice language has a solution to this problem that would make a nifty addition to Java.
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=5030232
January 20th, 2006 at 5:38 pm
I’m afraid that I don’t agree that this is a bug (or a problem). Enums, either from the typesafe enum pattern or from Java 5 are not basic types like int and float that always have some value. They are object references that are either null or refer to an object of the appropriate type. That enums in C/C++ always have a value is a side effect of their implementation as int’s. As soon as you start using the typesafe enum pattern in C++ you are implementing your Enums as pointers and a null pointer is always a possibility. If the contract for a setter is that it is never passed a null, or the contract for a getter is that it never returns one you must throw an exception for violation of the contract (or change the contract).
January 21st, 2006 at 3:07 pm
Throwing a documented exception for an invalid argument at runtime is a good thing. What’s wrong with catching some of those exceptions at compile time too?
August 24th, 2011 at 1:01 am
great post, here is another good link related to enum in java