Are you using Strings to identify types uniquely?

I've seen this happen in multiple codebases. Someone wants to create the concept of a unique Type - and they end up using Strings to do it instead of objects. The ugly consequences:
  • You're doing equality checks on Strings. Strings are not unique. This means that it is entirely possible for two distinct types to end up with the same string values through developer error.
  • Using strings tempts people to utilize them for things other than equality checks. They usually end up being used interchangeably with fields in that Constants class of which every codebase has at least one.
Let me illustrate.

Let's say we're working on code which has something to do with bikes. We have Hondas, Suzukis and Ducatis in the domain. We also have a Bike class which needs to have a type field - an instance of this class can be of type Honda, Ducati or Suzuki. We have a few choices:
  • Create subclasses
  • Introduce the concept of a type
  • Realize that magic numbers/strings/types are code smells and shouldn't be there in the first place

The last option is the nice one, but in the real world, it isn't always possible.
So let's say that the behaviour of the Bike class doesn't change from one type of Bike to another. This makes subclassing a bad idea. So someone introduces a BikeType class. And the next thing you know, you have fields like
public static String HONDA_BIKE = "honda";
showing up.

Then scope changes to include Cars so you figure you'll add a Car class. Next thing someones created CarType and put in
public static String HONDA_CAR = "honda";
Then someone else decides it's time to refactor to extract a Vehicle class. Soon you have
public static String HONDA_VEHICLE = "honda";

Since all these classes have a method
boolean isOfType(String)
you'll soon see all sorts of bugs introduced by developer error.
For example
Vehicle thisOne = new Bike(HONDA_BIKE);
Vehicle thatOne = new Car(HONDA_CAR);
thisOne.getType().equals(thatOne.getType())

now returns true.
This sounds like a silly thing to do, but can happen all the time in a big codebase with lots of developers working on it.

If you can't avoid types, the simplest thing to do is to go with an instance of Object. All object instances have a unique hashCode. Use this to create unique types. So now you have
public static Object HONDA_BIKE = new Object();
public static Object HONDA_CAR = new Object();
public static Object HONDA_VEHICLE = new Object();
All three are now unique and even better, nobody is going to use them to, say, populate the UI.

Once you've gotten to this point, you should be looking to convert these Object instances to concrete classes with some behaviour in them. You wouldn't need discrete types if you weren't making decisions based on them and this decision making logic would be a good place to start refactoring.

5 comments:

Ricky Clarkson said...

Use enums instead. That's why they exist. Also, you have a mistake in the last code. You cannot assign an Object to a String.

Unknown said...

Whoops. My bad about the code. Fixed it.

Anonymous said...

And thats the beauty of symbols in ruby

Chris Stevenson said...

Ricky, enums are ok if you are in Java. Don't use enums in C# because they are not typesafe. They compile down to magic numbers. There is a really good discussion of the Type Safe enum pattern in Joshua Bloch's Effective Java. The sample chapter here contains it: http://developer.java.sun.com/developer/Books/effectivejava/Chapter5.pdf - it is "Item 21: Replace enum constructs with classes"

Unknown said...

I wrote this after something I saw while working on a GWT codebase - which is Java 1.4 - I'm afraid enums didn't even occur to me. My bad.