- 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'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 likepublic 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 inpublic static String HONDA_CAR = "honda";
Then someone else decides it's time to refactor to extract a
Vehicle
class. Soon you havepublic 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 havepublic 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:
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.
Whoops. My bad about the code. Fixed it.
And thats the beauty of symbols in ruby
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"
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.
Post a Comment