Enum, switch, and a null

[Wojtek]

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.

 

[Daniel Pitts]

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.

<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]

....
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]

Patricia Shanahan wrote :

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

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 = FoNE; // 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...

 

[Lew]

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>

 

[Lew]

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>

 

[Wojtek]

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.

 

[Mike Schilling]

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>

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]

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.

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.printlnLjava/lang/StringV
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.printlnLjava/lang/StringV
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.printlnLjava/lang/StringV
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

 

[Roedy Green]

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.

 

[Stefan Ram]

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]

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.

 

[Daniel Pitts]

Daniel Pitts wrote :


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.

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.

 

[Daniel Pitts]

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();

 

[Wojtek]

Daniel Pitts wrote :

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.

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).

 

[Mike Schilling]

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

 

[Daniel Pitts]

Daniel Pitts wrote :






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).

So, if you add a new enum value, you don't have to update all of the
switch statements in 100 places?

 

[Roedy Green]

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

 

[Daniel Pitts]

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.

Seehttp://mindprod.com/jgloss/project/scid.html

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]

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.