To the tune of Iron Maiden’s The Evil That Me Do.
I keep coming across this (anti)pattern, and this time it annoyed me enough that I wanted to write in more detail about it.
Object’s contract
Let’s kick off by examining the documentation of Object’s default toString(). N.B The docs on Object are actually very good; I recommend reading the whole lot while you’re there.
/**
* Returns a string representation of the object. In general, the
* <code>toString</code> method returns a string that
* "textually represents" this object. The result should
* be a concise but informative representation that is easy for a
* person to read.
* It is recommended that all subclasses override this method.
*
* The <code>toString</code> method for class <code>Object</code>
* returns a string consisting of the name of the class of which the
* object is an instance, the at-sign character `<code>@</code>', and
* the unsigned hexadecimal representation of the hash code of the
* object. In other words, this method returns a string equal to the
* value of:
*
* getClass().getName() + '@' + Integer.toHexString(hashCode())
*
* @return a string representation of the object.
*/
public String toString() {
return getClass().getName() + "@" + Integer.toHexString(hashCode());
}
So, a few interesting points to mention in there. First off:
“Returns a string representation of the object.”
This is the contract of toString : you call it, you get something that represents the object you’re calling it on. Debuggers make use of this by implicitly toStringing variables in the current scope, and it’s very helpful. Presumably that’s why we then get the later advice: “It is recommended that all subclasses override this method.”
An abuse of toString
Now you know about toString, let’s examine a particular abuse of it.
final String requestBody =
new JsonMessageBuilder("heartbeat")
.addValue(NodeName.TOKEN, "PC Nick Rowan").toString();
Here, toString is used in the StringBuilder/StringBuffer style; i.e toString is a synonym for build. Two (rhetorical) questions
- Does that fulfil
Object‘s contract? - Is there a name that works better?
Or, both questions combined: Why isn’t that method called build?
Other problems
Instincts aside, it turns out that this overriding of toString also creates some difficulties across a codebase in general. It’s difficult to tell at which points we want the built String for a business reason or for debug output. Plenty of libraries allow you to get into the bad habit of passing them an object that they implicitly toString (I’m looking at you, log4j).
Try a ‘find usages’ on an overridden toString method in your preferred IDE. Good luck working out which of those occurrences actually involve your object; particularly if the object in question implements one or more interfaces.
Theoretical alternative
If this were any other method, we might be suspicious that a concrete override was at the heart of the design. How could we change java to ameliorate this?
Well, the default implementation isn’t that useful. It just tells us the class name and the hashCode of our instance. We can probably lose this method completely, both pieces of information are available separately. We can discuss the problems with hashCode (more concrete overrides, odd name for default implementation) another day.
If we want to let the debugger know about a Stringy description of ourselves, why not be explicit, and use something like hamcrest’s SelfDescribing?
Side note: the implementations of appendValue in Description allow passing in an arbitrary Object in. Imagine my disgust.
Conclusion
Don’t override toString to provide business logic.
Corollary: alarms should sound as soon as you start writing tests for a class’s toString. Move that functionality to a method that better describes the behaviour you want.
This post was brought to you by the following reasonably reliable general principles
- Explicit is better than implicit
- Encode constraints in types, not conventions
- Depend on interfaces, not implementations