Java Forum / General / September 2007
Enum, switch, and a null
Wojtek - 27 Aug 2007 23:56 GMT Given the following (bunch of stuff missing for clarity): ---------------------------------------
enum Foo { ONE, TWO; }
...
switch(getFoo()) { default: ONE: doOne(); break;
TWO: doTwo(); break; } ---------------------------------------
If getFoo() returns a null, then an exception occurs.
Should not the default case be processed? It should be the equivalent of a "not found" value, I would think.
 Signature Wojtek :-)
Daniel Pitts - 28 Aug 2007 00:06 GMT > Given the following (bunch of stuff missing for clarity): > --------------------------------------- [quoted text clipped - 27 lines] > -- > Wojtek :-) <SSCCE id="broken" pattern="switchOnTypeToken"> public class TestEnums { enum Test{ ONE, TWO }
public static void main(String[] args) { switch (getFoo()) { case ONE: System.out.println("One"); break; case TWO: System.out.println("Not one"); break; default: System.out.println("Other"); } }
private static Test getFoo() { return null; } } </SSCCE> Wow! I would have expected the same thing as you did.
Although, Switch is usually an antipattern. You should consider adding a value to your enum type for Other, and also adding methods to your enum to replace switch branches.
<example id="fixed" pattern="polymorphicStrategyOrState" fixes- SSCCE="broken"> public class TestEnums { enum Test{ ONE { public void doFoo() { System.out.println("One"); } } , TWO { public void doFoo() { System.out.println("Not one"); } }, UNDECIDED { public void doFoo() { System.out.println("Other"); } };
public abstract void doFoo(); }
public static void main(String[] args) { getFoo().doFoo(); }
private static Test getFoo() { return Test.UNDECIDED; } } </example>
Patricia Shanahan - 28 Aug 2007 00:13 GMT >> Given the following (bunch of stuff missing for clarity): >> --------------------------------------- [quoted text clipped - 19 lines] >> >> --------------------------------------- ...
> private static Test getFoo() { > return Test.UNDECIDED; > } > } > </example> Although I agree that switch is usually an anti-pattern, adding UNDECIDED as a Foo value does also solve the switch problem.
Using null as a non-error return from getFoo() suggests that there is a missing Foo value, the one for whatever condition is represented by null.
Patricia
Wojtek - 28 Aug 2007 00:45 GMT Patricia Shanahan wrote :
>>> Given the following (bunch of stuff missing for clarity): >>> --------------------------------------- [quoted text clipped - 33 lines] > > Patricia An of course it was.
getFoo() is a static method whose value is set during application initialization from a property file. If the value is missing or invalid, then the application throws an exception, and we never actually reach any switch statements.
This all came about because I DID have a NONE in the enum definition. But when I turned on the compiler warning to report missing switch/case when using a enum, then all sorts of warnings appeared (only for this one enum). So rather than put in the NONE case in all the switch's, I simply removed it from the enum (and the single place it was being used).
Remember, it would NEVER have a NONE value past initialization.
And then the application would not start, and it took me quite a while to figure it out :-( and find the ONE place where it WAS being used before the application finished initializing.
The fix is quite simple:
----------------------- Foo foo = getFoo();
if ( foo == null ) foo = Foo.ONE; // default!
switch(foo) -----------------------
And of course this is a run-time error, so the compiler does not catch it, yet the error occured because I wanted to make the compiler happy.
Oh the irony...
 Signature Wojtek :-)
Lew - 28 Aug 2007 00:57 GMT > switch ( expression-returning-null ) throws NPE While the JLS should address this explicitly, consider that the history of switch() is to switch off integral primitive types, which cannot have a null value. Another hint comes from the fact that the case labels must be constant expressions, and a "static final Integer" doesn't work as a case value.
In fact, if the switch expression is, say, Integer, a null value also will throw an NPE:
<sscce> public class Switcheroo { public static void main(String[] args) { Integer sw = null; switch ( sw ) { case 0: System.out.println( "zero" ); break; default: System.out.println( "not zero" ); break; } } } </sscce>
 Signature Lew
Mike Schilling - 28 Aug 2007 01:46 GMT >> switch ( expression-returning-null ) throws NPE > [quoted text clipped - 25 lines] > } > </sscce> Which you and I would both expect, since we know that
Integer x; switch(x) ...
is short for
Integer x; switch (x.intValue()) ...
I wonder, then, is
Enum e; switch(e)...
short for
Enum e; switch (e.ordinal())...
(Checks...) Yes it is, which explains the NPE, but geez, that's fragile. Change the order of the enum constants, forget to recompile the world, and your switches do entirely the wrong thimg.
Zig - 28 Aug 2007 03:24 GMT > Which you and I would both expect, since we know that > [quoted text clipped - 20 lines] > and > your switches do entirely the wrong thimg. You've under-estimated just how much time Sun spends on ensuring Binary compatability.
Given:
<sscce> package pkg1;
public class EnumDisassembly { public static Letter getLetter() { return Letter.A;} public static void main(String[] args) { switch (getLetter()) { case A: System.out.println("ABC"); break; case B: System.out.println("BCD"); break; case C: System.out.println("CDE"); } } enum Letter {A, B, C} } <sscce>
You will notice not 2, but 3 classes (I'm using jdk1.6.0_02 for reference)
EnumDisassembly.class, EnumDisassembly$Letter.class, and EnumDisassembly$1.class
Take a peek at our EnumDisassembly.main(), and look at the first line!
public static void main(java.lang.String[]); Code: 0: getstatic #3; //Field pkg1/EnumDisassembly$1.$SwitchMap$pkg1$EnumDisassembly$Letter:[I 3: invokestatic #4; //Method getLetter:()Lpkg1/EnumDisassembly$Letter; 6: invokevirtual #5; //Method pkg1/EnumDisassembly$Letter.ordinal:()I 9: iaload 10: tableswitch{ //1 to 3 1: 36; 2: 47; 3: 58; default: 66 } 36: getstatic #6; //Field java/lang/System.out:Ljava/io/PrintStream; 39: ldc #7; //String ABC 41: invokevirtual #8; //Method java/io/PrintStream.println:(Ljava/lang/String;)V 44: goto 66 47: getstatic #6; //Field java/lang/System.out:Ljava/io/PrintStream; 50: ldc #9; //String BCD 52: invokevirtual #8; //Method java/io/PrintStream.println:(Ljava/lang/String;)V 55: goto 66 58: getstatic #6; //Field java/lang/System.out:Ljava/io/PrintStream; 61: ldc #10; //String CDE 63: invokevirtual #8; //Method java/io/PrintStream.println:(Ljava/lang/String;)V 66: return
LineNumberTable: line 8: 0 line 9: 36 line 10: 47 line 11: 58 line 12: 66
}
And, looking at our synthetic $1 we see:
Compiled from "EnumDisassembly.java" class pkg1.EnumDisassembly$1 extends java.lang.Object{ static final int[] $SwitchMap$pkg1$EnumDisassembly$Letter;
static {}; Code: 0: invokestatic #1; //Method pkg1/EnumDisassembly$Letter.values:()[Lpkg1/EnumDisassembly$Letter; 3: arraylength 4: newarray int 6: putstatic #2; //Field $SwitchMap$pkg1$EnumDisassembly$Letter:[I 9: getstatic #2; //Field $SwitchMap$pkg1$EnumDisassembly$Letter:[I 12: getstatic #3; //Field pkg1/EnumDisassembly$Letter.A:Lpkg1/EnumDisassembly$Letter; 15: invokevirtual #4; //Method pkg1/EnumDisassembly$Letter.ordinal:()I 18: iconst_1 19: iastore 20: goto 24 23: astore_0 24: getstatic #2; //Field $SwitchMap$pkg1$EnumDisassembly$Letter:[I 27: getstatic #6; //Field pkg1/EnumDisassembly$Letter.B:Lpkg1/EnumDisassembly$Letter; 30: invokevirtual #4; //Method pkg1/EnumDisassembly$Letter.ordinal:()I 33: iconst_2 34: iastore 35: goto 39 38: astore_0 39: getstatic #2; //Field $SwitchMap$pkg1$EnumDisassembly$Letter:[I 42: getstatic #7; //Field pkg1/EnumDisassembly$Letter.C:Lpkg1/EnumDisassembly$Letter; 45: invokevirtual #4; //Method pkg1/EnumDisassembly$Letter.ordinal:()I 48: iconst_3 49: iastore 50: goto 54 53: astore_0 54: return Exception table: from to target type 9 20 23 Class java/lang/NoSuchFieldError
24 35 38 Class java/lang/NoSuchFieldError
39 50 53 Class java/lang/NoSuchFieldError
LineNumberTable: line 8: 0
}
So, it appears that everytime you use a switch on an Enum, a new synthetic class is created for you to load a $SwitchMap$ field, and thus populate the correct ordinals. Thus, changing an Enum should be safe between full compilations, so long as you don't delete a constant in someone else's switchmap. Though, each class that uses a switch on an Enum create a new synthetic class is a glaring downside to using the enhanced switch :/
HTH,
-Zig
Mike Schilling - 28 Aug 2007 17:37 GMT >> Which you and I would both expect, since we know that >> [quoted text clipped - 23 lines] > You've under-estimated just how much time Sun spends on ensuring Binary > compatability.
> [snip example and disassembly]
> So, it appears that everytime you use a switch on an Enum, a new synthetic > class is created for you to load a $SwitchMap$ field, and thus populate > the correct ordinals. Thus, changing an Enum should be safe between full > compilations, so long as you don't delete a constant in someone else's > switchmap. Though, each class that uses a switch on an Enum create a new > synthetic class is a glaring downside to using the enhanced switch :/ Thanks for sharing; I had had no idea it worked that way. It's impressive, though I'm not sure I'm impressed positively :-)
Lew - 28 Aug 2007 00:54 GMT > Although I agree that switch is usually an anti-pattern, adding > UNDECIDED as a Foo value does also solve the switch problem. > > Using null as a non-error return from getFoo() suggests that there is a > missing Foo value, the one for whatever condition is represented by null. While the JLS should address this, consider that the history of switch() is to switch off integral primitive types, which cannot have a null value. The extension to use the wr
A hint comes from the fact that the case labels must be constant expressions, and "static final Integer" doesn't work.
In fact, if the switch expression is, say, Integer, a null value also will throw an NPE:
<sscce> public class SwitchClasses { public static void main(String[] args) { Integer sw = null; switch ( sw ) { case 0: System.out.println( "zero" ); break; default: System.out.println( "not zero" ); break; } } } </sscce>
 Signature Lew
Wojtek - 28 Aug 2007 00:59 GMT Daniel Pitts wrote :
> You should consider ... and also adding methods to > your enum to replace switch branches. I do not like placing business logic into what is basically a selector.
This particular enum selects which database engine is being used. It is used in 100's of abstract classes. Which would mean I would need either 100's of methods, or a really complex factory class.
 Signature Wojtek :-)
Daniel Pitts - 28 Aug 2007 15:44 GMT > Daniel Pitts wrote : > [quoted text clipped - 9 lines] > -- > Wojtek :-) If its being used in 100's of abstract classes, that means you have 100's of switch statements. Wouldn't polymorphism be the better approach by far? Perhaps using enum isn't exactly what you intended.
Instead of getDatabaseSelector(), you could write one method that is called getDatabaseEngine(), and it will return to you an implementation of your database engine.
Wojtek - 28 Aug 2007 16:16 GMT Daniel Pitts wrote :
>> Daniel Pitts wrote : >> [quoted text clipped - 17 lines] > called getDatabaseEngine(), and it will return to you an > implementation of your database engine. I use an enum to enforce the type. I use a switch because it is more "elegant" than an if/else tree. I have 100's because each use case is logically separated (as much as possible) from every other use case.
It may not wring every last bit of performance out of the CPU, it may not be 100% OO, but it IS easy to read (understand), easy to implement (new UC's), and easy to maintain (minimal interaction between UC's).
 Signature Wojtek :-)
Daniel Pitts - 29 Aug 2007 01:41 GMT > Daniel Pitts wrote : > [quoted text clipped - 30 lines] > -- > Wojtek :-) So, if you add a new enum value, you don't have to update all of the switch statements in 100 places?
Roedy Green - 01 Sep 2007 13:33 GMT On Tue, 28 Aug 2007 17:41:11 -0700, Daniel Pitts <googlegroupie@coloraura.com> wrote, quoted or indirectly quoted someone who said :
>So, if you add a new enum value, you don't have to update all of the >switch statements in 100 places? On the other paw, you at least can see how you handled all the other enums when you write this code. You are more likely to get each one correct even if you are likely to forget one.
What happens if you leave off default with enums? Is the compiler smart enough to warn you if you leave out an enum? Is it smart enough not to complain if you do provide them all?
In a SCID I suggest you should be able to transform your code so you can see it comparing how all enums implemented a method, and how a given enum implemented all its method. My studying the code from both views you are more likely to get is right.
See http://mindprod.com/jgloss/project/scid.html
 Signature Roedy Green Canadian Mind Products The Java Glossary http://mindprod.com
Daniel Pitts - 01 Sep 2007 17:53 GMT On Sep 1, 5:33 am, Roedy Green <see_webs...@mindprod.com.invalid> wrote:
> On Tue, 28 Aug 2007 17:41:11 -0700, Daniel Pitts > <googlegrou...@coloraura.com> wrote, quoted or indirectly quoted [quoted text clipped - 20 lines] > Roedy Green Canadian Mind Products > The Java Glossaryhttp://mindprod.com The compiler may issue a warning if your switch doesn't have the enum branches. The compiler WILL issue an ERROR if you don't implement an abstract method in a concrete class.
If you need to study all other implementations in order to get your implementation correct, that seems to me to be a bad design. The infrastructure should take care of all the complicated bits, and just ask the polymorphic class to handle the differing details.
Wojtek - 10 Sep 2007 18:48 GMT Daniel Pitts wrote :
> So, if you add a new enum value, you don't have to update all of the > switch statements in 100 places? Yes, and the compiler tells me where they are.
I am using this enum to select which database engine is being used. I have an abstract class which has the switch statement. It instantiates the concrete class for the data base engine being used. The concrete class has the correct SQL syntax for that engine to do whatever updates/queries need to be done for the use case.
In the simplest case, such as the login, the SQL class looks like: ------------------------------------- abstract class SQL extends DBEngineBase { SQL( TransactionCommit setAutoCommit ) throws SQLException { super( setAutoCommit ); }
abstract void login( Data data ) throws SQLException;
static SQL getInstance( TransactionCommit setAutoCommit ) throws SQLException { SQL sql = null;
switch (Database.getDBType()) { case MS_SQL: sql = new SQL_Microsoft( setAutoCommit ); break; }
return sql; } }
------------------------------------- The SQL_Microsoft is: ------------------------------------- class SQL_Microsoft extends SQL { SQL_Microsoft( TransactionCommit setAutoCommit ) throws SQLException { super( setAutoCommit ); }
@Override void login( Data data ) throws SQLException { // bunch of code } } ------------------------------------- and to get this going, in the Command class: ------------------------------------- SQL sql = SQL.getInstance( SQL.TransactionCommit.AUTOMATIC_COMMIT );
sql.login( data ); -------------------------------------
So the Command class does not know what DB engine is being used (nor should it). It simply gets an instance, and the SQL class decides which engine is to be used.
If I add a new supported database engine, all the SQL code will need to be written for that engine. The compiler will tell me where I am missing a case for the new enum.
If I tried to put all the SQL methods for each use case in an enum, then the enum would need to know about each use case, and moreover I would need hundreds of methods within the enum.
 Signature Wojtek :-)
Lew - 11 Sep 2007 00:36 GMT > switch (Database.getDBType()) > { [quoted text clipped - 4 lines] > > return sql; Another approach is to instantiate a Map <DBType, SQL> to retrieve the desired SQL (or a Class<? extends SQL>), instantiated from a descriptor or other resource.
SQL sql = dbMap.get( Database.getDBType() );
 Signature Lew
Wojtek - 11 Sep 2007 00:42 GMT Lew wrote :
>> switch (Database.getDBType()) >> { [quoted text clipped - 10 lines] > > SQL sql = dbMap.get( Database.getDBType() ); So then the get would return an already instantiated class? What about threading? Would not all threads then get the same instance?
This is part of a Web app. Lots of threads...
Or is dbMap created for each use. That means I would be creating an extra object (the map), plus an extra object for each DB engine, but only using one of the engines for the life of the app.
 Signature Wojtek :-)
Daniel Pitts - 11 Sep 2007 00:59 GMT > Lew wrote : > [quoted text clipped - 24 lines] > -- > Wojtek :-) Hey, use Hibernate, that'll solve all your problems! :-)
If you change your DBType enum to be an interface or class with the a method: abstract SQL getSQLInstance(TransactionCommit setAutoCommit);
class MS_SQL_DBType implements DBType { public SQL getSQLInstance(TransactionCommit setAutoCommit) return new SQL_Microsoft( setAutoCommit ); } }
class MySQL_DBType implements DBType { public SQL getSQLInstance(TransactionCommit setAutoCommit) return new SQL_MySSQL( setAutoCommit ); } }
etc... and so forth.
Add a method to your DBType interface for every switch statement you currently have.
Does that approach make sense now?
Lew - 11 Sep 2007 01:16 GMT Wojtek wrote:
>> So then the get would return an already instantiated class? What about >> threading? Would not all threads then get the same instance? That's why I mentioned the Class<? extends SQL> option, duhy.
>> This is part of a Web app. Lots of threads... >> >> Or is dbMap created for each use. That means I would be creating an >> extra object (the map), plus an extra object for each DB engine, but >> only using one of the engines for the life of the app. Create it with the scope you desire. You only need to instantiate what you use, but at least you won't have miles of hard-coding to deal with each situation. Plus, you'd only instantiate the Class objects for the Map that are specified in your deployment, so you would only have "an extra object for each DB engine" that you want, not for all the world.
It's just an alternative, but the specific objections you raise are pretty minor and easily handled. Personally I'll take an unnoticeable reduction in run-time performance any time if it creates more maintainable code, which moving things out of hard-coding generally does.
 Signature Lew
Wojtek - 11 Sep 2007 02:33 GMT Daniel Pitts wrote :
>> Lew wrote : >> [quoted text clipped - 49 lines] > > Does that approach make sense now? Each use case has a SQL/SQL_Microsoft pair. So for updating a simple helper table such as StreetType (Street, Avenue, etc) I have the methods getRow(), saveRow(), and newRow().
This has default visibility (package level) and is only used within that package.
SQL and SQL_Microsoft are specific to the package they are in. There is NOT a huge monolithic class with 100's of methods to handle each possible use case. Each SQL_XX only has the methods required for its own use case. The login example is the entirety of those classes. No other methods.
I am not convinced. Each example given seems to assume that there is one and only one SQL_Microsoft class in the entire system. There is not. A partial package tree would look like:
app.uc.streetType Servlet Command SQL SQL_Microsoft app.uc.stateProvince Servlet Command SQL SQL_Microsoft app.uc.person.login Servlet Command SQL SQL_Microsoft app.uc.person.edit Servlet Command SQL SQL_Microsoft
and so on. If in the future, the PHB's decide we need other engines supported, then it might look like:
app.uc.streetType Servlet Command SQL SQL_Microsoft SQL_MySQL SQL_Oracle SQL_Paradox
 Signature Wojtek :-)
Roedy Green - 28 Aug 2007 06:03 GMT >If getFoo() returns a null, then an exception occurs. It could have been defined that, way but it wasn't. null means "missing value". Default means "other value".
The problem is similar to the picky distinction between empty and null strings. see http://mindprod.com/jgloss/void.html
Similarly the distinction between an null and empty HashMap.
If I were redesigning Java from scratch I would think long and hard about some convention so you did not have to write explicit code all over the place to deal with nulls, and perhaps do away with the notion of null altogether in some way.
 Signature Roedy Green Canadian Mind Products The Java Glossary http://mindprod.com
Stefan Ram - 28 Aug 2007 06:24 GMT >The problem is similar to the picky distinction between empty >and null strings. see http://mindprod.com/jgloss/void.html In everyday life, the distinction between an empty glass and no glass is not considered to be that picky.
Lew - 28 Aug 2007 13:41 GMT >> The problem is similar to the picky distinction between empty >> and null strings. see http://mindprod.com/jgloss/void.html > > In everyday life, the distinction between an empty > glass and no glass is not considered to be that picky. One can hurl an empty glass into the fireplace.
 Signature Lew
Daniel Pitts - 28 Aug 2007 15:45 GMT > >The problem is similar to the picky distinction between empty > >and null strings. seehttp://mindprod.com/jgloss/void.html > > In everyday life, the distinction between an empty > glass and no glass is not considered to be that picky. Yes, what if you only care about the drink :-)
I agree though. null DOES have a place. I think the solution that Roedy is after would be to (by convention) create null-token objects and enforce that none of his code ever produces null.
public Map<String, String> getStuff() { if (stuff == null) { return Collections.emptyMap(); } return stuff; }
public String getThing() { return thing == null ? "" : thing; }
private MyStrategyObject strategy = new NoStrategyObject();
Free MagazinesGet these publications absolutely FREE for up to 12 months. There are no hidden fees and no obligation. Simply choose a title, complete the application form and submit it. Read more ...
|
|
|