Never Use toString() for Behaviour | Hacker News Reader

Never Use toString() for Behaviour

java.christmas

69 pointssindrebn49 comments

>If left unoverridden, it only yields a description of it’s class and location in memory

It contains the hashCode(), not the memory location. The default implementation of hashCode could use a memory location to generate the hash, however that isn't the case in all implementations. OpenJDK seems to have used an RNG by default in the past and currently derives a value from the thread state[1]

[1]https://srvaroa.github.io/jvm/java/openjdk/biased-locking/20...

for most implementations it would not be possible to use the location in memory because most java garbage collectors will move stuff around in memory. they could use the location initially and cache it afterwards but this would seem tricky to do without breaking the hash code contract.

I think the code also has the option to read and store the address. However I also think it would have other problems. Heaps with less than 4GB size would put a limit on possible hash code values and object alignment requirements would make it even worse, so you never get use out of the full 32 bit hashCode. Then you have generational GCs and if I understand those correctly you could end up with a lot of objects that started out at the same address and if you call hashCode on them before they are moved to a long lived generation you almost guarantee a large amount of hash collisions down the line.

Thanks for the reply - the author (who doesn't have a user here) agreed completely, and the article has been changed to reflect that.

Agree in general, but note that Java itself does not follow this rule: StringBuilder's and StringWriter's toString methods use toString for behavior — creating a string from accumulated chars, and I'm fine with that.

This also definitely does not apply to .NET where there are many classes that override ToString for meaningful, well-documented behavior (StringBuilder is one, funnily enough). It's best to check the docs or look at IntelliSense before applying this rule in a blanket fashion.

That said, it does seem like Microsoft has since moved away from overriding ToString for such things. In all honesty, Object.ToString was probably a mistake to begin with.

There's nothing wrong with returning something well-specified from toString, but I agree this should never be relied upon. And even if toString actually return something well-specified, the same functionality should be available as a distinct method.

Any form of (semi-)automatic SomeObject -> String should be treated as a convenience for the developer to avoid getting a non-descriptive memory location reported.

Having the automatic toString isn't the problem, but maybe the name of the method is. It could be called "asDebugHelpRepresentation" or something more descriptive.

The typical usage for StringBuilder tends to make it pretty obvious you're using a StringBuilder and everybody expects you will convert to a string at the end. Usually StringBuilders are short lived and all the calls and the declaration are close to each other.

Ok, but then the rule is not to use toString() for behavior unless it "makes sense" to do so, which is not a rule at all.

I think that misses my point. It's an established pattern recognizable to anyone who has read or written a lot of code. And locality of use and declaration is high. The danger of it being mixed up with another type in a refactor in faraway code is low.

Whereas in the general case, where code can be sparse and types can be more freely altered, you could very easily change the type of something and not realize that a ToString() call on that type lurks somewhere else in the code base.

This also goes for logging and even string-ification in a lot of popular Java libraries. It's certainly good to write methods like getName() if you foresee an issue with changes down the line, but precisely because toString is universal, it may well be invoked somewhere outside what you're writing.

Java's eccentric uncle Objective-C called this method "description" instead of "toString". That was a better name IMO because it avoids the suggestion that it's some kind of canonical string representation.

Yeah, it really would be helpful if the method name made that clear.

The language documentation does make it clear what toString() is for. And it's convenient to be able to (for example) stick objects and strings together with the "+" operator and have conversions automatically happen.

But (understandably) not everyone reads the language documentation cover to cover before they start writing Java code, so you can't count on that making it obvious.

java.lang.Object has a few methods though. At least those should be known when one starts writing productive code. Otherwise one produces even worse bugs than relying on toString behavior (equals and hashCode, god forbid).

Cover to cover would mean understanding the memory model and maybe garbage collection. That's too far certainly but a few basics would do no harm. toString is surely one of them.

I'd hazard a guess that 0.1% of Java developers are aware that on some operating systems Object.wait() can awaken spuriously. Heck, it wasn't even in the Javadoc until 1.5. And don't even get me started on how you should handle an InterruptedException.

Most Java developers I know consider rolling your own multi-threading scheme about as wise as rolling your own cryptography, and just pretend notify/notifyAll/wait don't even exist.

Ironically, that's one of the few things I do know off the top of my head about Object.wait(). I'm far from an expert on Object's threading methods, but "this will burn you, don't try it" is becoming more pervasive than "here's how to do this". Which is almost certainly the right order to learn those things in.

Most Java developers don't know what to do when something can throw InterruptedException. I'd argue that bugs caused by broken code that catches this exception are much worse than what can happen when not using toString() correctly.

Ah, Objective-C: Combining the blazing-fast speed of Smalltalk with the famous memory-safety of C.

We use lombok, and the toString annotation is immensely helpful for logging and avoiding this type of thing.

How does Lombok help to avoid this?

Lomboks annotation @ToString will automatically generate the typical overridden toString() method based on the fields.

Even Object.toString could do this using reflection. If you're worried about the performance, override it. I never understood why this is not the case (doesn't Haskell do something similar?).

I just tried this in Java and I ran quickly into circularity issues (one of the fields has an indirect reference to the same object, say a list which the current instance is part of). How does Lombok cope with that?

It's not that easy as I thought (see my sibling comment).

> If you used the returned Country to construct an API request including the name of the country, this will now produce strange countries named as “Optional[“Norway”]”

I am not too familiar with java, but should’t you call get() from optional to get Country from Optional<Country> and toString of Country will behave as expected.

The point is that if you change the type, existing calls to country.toString() will still compile. (Normally, fixing compile errors is a fairly reliable way to do this kind of refactoring.) That means you can’t guarantee to catch them all, so there’s a risk of modified behavior escaping into production.

As the sibling comment says, the post's advice is really about making your code refactoring-friendly with regard to type checking.

You generally would not want to call '.get()' without an 'isPresent()' check because it will throw a NoSuchElementException if the Optional is empty. This is a foreseeable case, so most would like to handle it somewhat explicitly.

Today, it is idiomatic to use '.map' to transform it:

    Optional<String> foo = optionalCountry.map(Country::getName);

You can also throw a domain-specific exception from that point:

    Optional<String> foo = optionalCountry
        .map(Country::getName)
        .orElseThrow(...);

orElseThrow does return a String, not an Optional. You also could use orElse(null) or orElse("null") depending on the use case (eg. using as a part of toString).

I'm useless without a compiler to check my types!

Cool article! It's a simple mistake to make when starting out - good to get a nice explanation of why it's a bad practice

Additionally, relying on `toString()` could cause problems when multiple different representations are desired.

Never use a language that puts a toString() implementation on every object regardless of whether or not that object implements toString()

The ability to convert any object to a string is very useful for debugging and most languages support this. Even in C it's frequently useful to print %p to identify objects.

Serialization should require involve separate APIs.

It should be opt-in though. A compiler-flag for debug builds or similar.

Why so, what are the downsides? It's not like it introduces any meaningful overhead at all, and there are plenty of situations where it is useful even in production (eg for logging in catchall "can't happen" top level exception handlers). Adding yet another compiler flag seems like a worse solution to me.

What you lose is the ability to model objects based on the behaviours that they can perform.

I want to be able to design objects that have toString() and I want to be able to design objects that don't have toString().

The same goes for clone(), hash(), getPtr(), getEndian(), equals(), toBytes(), toJson(), encrypt(), delete(), isNumeric(), toXml(), serializationVersion() or compare().

I suppose you could define toString() to throw an error, but obviously that's a messy hack in place of having that control. I can definitely appreciate the logic for making nothing intrinsic, and I can also see an argument for making equals() the only method of Object. (i.e. everything should have an identity function, but nothing more.)

Now I'm wondering... are there languages which formalize "implement with a throw" into some kind of explicit refusal to implement a method? Obviously there are method-sharing approaches other than inheritance, but I've never heard of "you must implement this, or explicitly choose not to".

I'm not quite sure what you mean, but it's standard in python if you have a method in a class that must be subclassed and overridden you can have the default raise a NotImplementedError

I was thinking about a condition that's specifically for "inherited but not supported", but that's pretty close to what I had in mind, yes.

Java doesn't have a language-level exception that's specifically for unusable methods, which can be a bit awkward. UnsupportedOperationException is the recommended answer, but it's relatively nonspecific as to why your operation wasn't supported. Apache's Commons extends that with NotImplementedException for cases where the call isn't definitionally impossible (e.g. adding to an unmodifiable map), but no implementation exists.

Which is why C# (and apparently other .NET constituents) has the excellent, but separate DebuggerDisplay API [0]. IIRC it also makes it possible to define a private implementation to communicate to clients that this pseudo-serialization is not part of the public API (like ToString() implicitly would be).

[0] https://docs.microsoft.com/en-us/visualstudio/debugger/using...

That %p approach sounds perfect.

If they wanted to make it worse, they could make it so you could call %s on any object, and if it wasn't defined, it would "conveniently" fall back to %p instead.

Then someone could could write an article called "Never Use %s for Behaviour"

Great advice to be unemployable.

Hah, such hate.

Lemme quote Goetz & Steele:

"Standard methods like toString and equals should not be forced on value classes, but should be customizable." (http://cr.openjdk.java.net/~jrose/values/values-0.html)

That was a complete waste of time.

To sum it up: don't rely on the format of a generic toString() method. Prefer country.getName() over country.toString(), even if toString() returns getName().

It's cool to read, but yeah, it's like reading Beginner's Java, lesson 3.

More like "beginning software architecture". While this is "common knowledge" after coding for years, it's actually hard-earned, battle-scarred knowledge. Sharing experience on how to design software is very useful for all the people just starting out.

Unlikely. Beginner's Java, lesson 3 is the reason people are using toString() erroneously in the first place.

> That was a complete waste of time.

I think that's a bit harsh. It's an article explaining simply, but effectively why you should probably not use toString(). Just because something can be summed up in a sentence does not make it a waste of time.

A complete waste of time for some, and very useful for others. I don't believe comments like this is constructive for anyone.

Yeah, I was a bit harsh. But they already posted an announcement post about their Christmas calendar: https://news.ycombinator.com/item?id=21700759 I don't see the need to go into individual posts on top of it.

In the end, I just think there are better channels for getting started with programming in a particular language then this site.

Reply on news.ycombinator.com