Written by Michael Whitaker
on July 16, 2020

Use Primitive Wrapper `toString` implementations

One of the more common minor bugs I’ve seen is caused by the use of String.valueOf. I’m not going to go as far as to say it shouldn’t be used (there are plenty of situations where it makes sense) but rather than it shouldn’t be used when a better alternative exists.

Consider the following function:

public String getCompositeIdentifier(Message message) {
  String uuidSuffix = UUID.randomUUID.toString();
  return String.valueOf(message.getId()) + "-" + uuidSuffix;
}

Few problems that you’re probably already seeing:

We have no idea what the type of message.id is in this case just by reading the code
If message.id is null we may get a value we didn’t expect.

But what happens when the type of message.id changes?

Since under the hood String.valueOf calls the objects toString implementation, and overloaded versions exist for all primitives, a change in type here won’t result in the compiler letting us know that something has changed.

One of the most common cases I see this in is when switching getters on nullable fields to return Optional (an admirable goal).

public class Message {
  private long id;
  public Message(long id) {
    this.id = id;
  }
  public long getId() {
    return id;
  }
}

In this case, String.valueOf will return an expected String representation of the long value. Get BlogAboutCode’s stories in your inbox

However if it turns out that id isn’t being set properly and defaulting to OL someone may wish to switch that to a nullable primitive wrapper type and enclose the getter in an Optional wrapper.

public class Message {
  private Long id;

  public Message(Long id) {
    this.id = id;
  }

  public Optional<Long> getId() {
    return Optional.ofNullable(id);
  }
}

Doing this would result in String.valueOf returning "Optional [101]" or "Optional.empty"if the id was101Lornull` respectively. Despite this significant change in behavior a compiler error would not be triggered.

This might not seem like such a big deal either, after all, you can just grep for all usages of this method in the code and make sure they aren’t using String.valueOf right? Well maybe, but that’s going to be a painful process, particularly in a large codebase, and during a code review if someone else made this change, there wouldn’t be a diff on any of the String.valueOf lines.

Using primitive wrapper methods

Now let’s say instead that we had used the primitive wrapper Long.toString helper method. The original code would look something like this:

public String getCompositeIdentifier(Message message) {
  String uuidSuffix = UUID.randomUUID.toString();
  return Long.toString(message.getId()) + "-" + uuidSuffix;
}

And the return value change in the getter would throw a compiler error, forcing the author to deal with the type (and in this specific case also forcing them to deal with the existence of that value).

public String getCompositeIdentifier(Message message) {
  String uuidSuffix = UUID.randomUUID.toString();
  return message.getId()
      .map(id -> Long.toString(id) + "-" + uuidSuffix)
      .orElseThrow(); //Or sensible default, etc
}

If this is followed throughout the codebase, people can refactor with a higher degree of confidence and code reviewers can more clearly view how the changes impact the rest of the system.

TL;DR

Instead of using String.valueOf on primitive types, consider using the primitive wrapper toString implementation to ensure the compiler is able to appropriately inform you when changes occur that break assumptions about the object you are operating on.

Top