Silly Java Enumeration Refactorings

This silliness all got started in AvoidFinalStringsForUniqueTypes, where ToddCoram proposed the following in place of ugly manifest strings.

class Direction {
private static class Heading {
  private String dir;
  Heading(String aDirection) {
dir = aDirection;
  }
  public String toString() {
return dir;
  }
}
public final static Heading NORTH = new Heading("NORTH");
public final static Heading SOUTH = new Heading("SOUTH");

public void setOrientation(Heading aHeading) {
  System.out.println("You are now Facing " + aHeading);
  if (aHeading == NORTH || aHeading == SOUTH) {
System.out.println("Head for the pole!");
  }
}
}

I noticed that the next logical step would be to ReplaceConditionalWithPolymorphism:

class Direction {
private static class Heading {
 void printSillyMessage() {}
}
public static class PolarHeading? extends Heading {
void printSillyMessage() {System.out.println("Head for the pole!");}
}
public final static Heading NORTH = new PolarHeading?() {
public String toString() {return "NORTH";}
};
public final static Heading SOUTH = new PolarHeading?() {
public String toString() {return "SOUTH";}
};
public final static Heading EAST = new Heading() {
public String toString() {return "EAST";}
};
public void setOrientation(Heading aHeading) {
  System.out.println("You are now Facing " + aHeading);
  aHeading.printSillyMessage();
}
}

Now these constants actually do something. Instead of being just constants, now they have functionality useful to the class they are defined within. And, conveniently, the actual Heading objects (aka singletons) once again contain no state (i.e., their name), for whatever that's worth.

But wait, there's more! We are supposed to DefineConstantsInInterfaces. Now we get:

class Direction {
static interface Heading {
class Basic {
  void printSillyMessage() {}
}
class Polar extends Basic {
  void printSillyMessage() {System.out.println("Head for the pole!");}
}
public final static Heading NORTH = new Polar() {
  public String toString() {return "NORTH";}
};
public final static Heading SOUTH = new Polar() {
  public String toString() {return "SOUTH";}
};
public final static Heading EAST = new Basic() {
  public String toString() {return "EAST";}
};
}
public void setOrientation(Heading aHeading) {
  System.out.println("You are now Facing " + aHeading);
  aHeading.printSillyMessage();
}
}

Then, a user of the constants need only implement Direction.Heading to get at NORTH.

Actually, I wonder if putting classes inside interfaces is a way to fake implementation MultipleInheritance. Any thoughts?

-- BillTrost

Doesn't seem to offer much. It just gives you some namespace flexibility. To get at the implementation of such a class, you can delegate to an object of that class or extend the class. Implementing the interface doesn't automatically do either. --KielHodges

---

I noticed that the next logical step would be to ReplaceConditionalWithPolymorphism

It would be more accurate to say, "From here, a possible refactoring step would be to ReplaceConditionalWithPolymorphism". There's no One True Direction to refactoring: sometimes you break things up, sometimes you put things together; sometimes you replace a branch with subclassing, sometimes you replace subclassing with a branch.

Which refactoring moves you should make depends on where you want to go. Right before you add a feature, you want to make things a little more general. Right after you add a feature, you want to get rid of redundancy, and to make the code easier to understand.

In the actual case above, it's hard to see why you'd want to replace a simple use of the contstants

 if (aHeading == NORTH || aHeading == SOUTH)

with a pretty skimpy new subclass. Among other problems, it's nowhere near as clear to me (and I think to most other people). A switch on all the values would be more motivating, and two or more such switches would probably motivate me, personally, to do something:

class Direction
private static class Heading {
  private String dir;
  Heading(String aDirection) {
dir = aDirection;
  }
  public String toString() {
return dir;
  }
}
public final static Heading NORTH = new Heading("NORTH");
public final static Heading SOUTH = new Heading("SOUTH");
public final static Heading EAST = new Heading("EAST");
public final static Heading WEST = new Heading("WEST");

public void setOrientation(Heading aHeading) {
  System.out.println("You are now Facing " + aHeading);
  if (aheading == NORTH)
System.out.println("You are moving toward Michigan");
  else if (aheading == SOUTH)
System.out.println("You are moving in the direction of Texas");
  else if (aheading == EAST)
System.out.println("You are moving closer to New Jersey");
  else if (aheading == WEST)
System.out.println("You are moving California-ward");
  if (aHeading == NORTH || aHeading == SOUTH) {
System.out.println("Head for the pole!");
  }
}
}

That something, though, would probably be more along the lines of adding a little extra state to each instance: a String with the value "toward Michigan", "in the direction of Texas", "closer to New Jersey", or "California-ward". (This is actually a very simple case of delegation (to a String); in general, you can use delegation instead of multiple inheritance.) The code would then look like:

public final static Heading NORTH = new Heading("NORTH", "toward Michigan");
// etc.

public void setOrientation(Heading aHeading) {
  System.out.println("You are now Facing " + aHeading);
  System.out.println("You are moving " + aHeading.getLongDescription());
  if (aHeading == NORTH || aHeading == SOUTH) {
System.out.println("Head for the pole!");
  }
}

...which I think is an improvement in both compactness and clarity. And if I didn't think that, I wouldn't do it.

Refactoring is a game. The refactorings are like the legal moves in chess: it helps to know them, but none of them are mandatory, and knowing them isn't the same as being a good chess player. You still need to set goals, use common sense, use uncommon sense, and otherwise engage your brain. --GeorgePaci

---

CategoryJava