- 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
Constantsclass of which every codebase has at least one.
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
Bikeclass 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
Bikeclass doesn't change from one type of Bike to another. This makes subclassing a bad idea. So someone introduces a
BikeTypeclass. And the next thing you know, you have fields like
public static String HONDA_BIKE = "honda";
Then scope changes to include Cars so you figure you'll add a
Carclass. Next thing someones created
CarTypeand put in
public static String HONDA_CAR = "honda";
Then someone else decides it's time to refactor to extract a
Vehicleclass. Soon you have
public static String HONDA_VEHICLE = "honda";
Since all these classes have a method
you'll soon see all sorts of bugs introduced by developer error.
Vehicle thisOne = new Bike(HONDA_BIKE);
Vehicle thatOne = new Car(HONDA_CAR);
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
Objectinstances 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.