What is the proper error message to supply to Google Guava's Preconditions.* methods?

Question

For example when using Preconditions.checkArgument, is the error message supposed to reflect the passing case or the failing case of the check in question?

import static com.google.common.base.Preconditions.*;

void doStuff(int a, int b) {
  checkArgument(a == b, "a == b");
  // OR
  checkArgument(a == b, "a != b");
}

Iridize

Answer 1

For precondition checks, stating the requirement in the exception detail message is more informative than simply stating the facts. It also leads to a lot more naturally reading code:

The documentation provides the following example:

This allows constructs such as

 if (count <= 0) {
   throw new IllegalArgumentException("must be positive: " + count);
 }

to be replaced with the more compact

 checkArgument(count > 0, "must be positive: %s", count);

Two things are of note:

• The example clearly states requirement, rather than the fact
  • "something must be right", instead of "something is wrong"
• By inverting the condition of the check, the entire construct read more naturally

Following that example, a better message would be:

checkArgument(a == b, "a must be equal to b"); // natural!

You can even go a step further and capture the values of a and b in the exception message (see Effective Java 2nd Edition, Item 63: Include failure capture information in detail messages):

checkArgument(a == b, "a must be equal to b; instead %s != %s", a, b);

The alternative of stating facts is less natural to read in code:

checkArgument(a == b, "a != b");              // awkward!
checkArgument(a == b, "a is not equal to b"); // awkward!

Note how awkward it is to read in Java code one boolean expression, followed by a string that contradicts that expression.

Another downside to stating facts is that for complicated preconditions, it's potentially less informative, because the user may already know of the fact, but not what the actual requirements are that is being violated.

Related questions

• Null check error message as “is null” or “was null”

Intitule

Answer 2

The documentation gives examples which make it very clear using words instead of symbols:

checkArgument(count > 0, "must be positive: %s", count);

Given that these are basically just going to be exception messages in logs (or debugging sessions), do whatever you think will make it clearest to anyone looking at problem, or anyone reading the code to understand the precondition.

For example, you could rewrite the above as:

checkArgument(count > 0, "count must be positive; was actually %s", count);

Strohbehn

In the example I gave, which do you prefer? – Iridize

@Brian: I don't like either of them, really. Neither of them show the values of a and b, and without either that or a meaningful message, I'd be loathe to assume too much. If I were forced to pick between them, I'd probably prefer the latter... but I'd really suggest using something more meaningful. – Strohbehn

Answer 3

Personally, for something like this I think I'd write

checkArgument(a == b, "a (%s) must equal b (%s)", a, b);

Cavernous

Answer 4

Often (most of the time) you can just use the checkArgument() method without the error message parameter, that is, not supplying the error message at all.

To make a good error message one needs to understand who will read the message. It is not the end-user; if the precondition fails it points most likely to a bug in the code. The text may explain the failed precondition, but in most cases it is of no use to the end-user, it is supposed to be a hint for the programmer. Most of the time, the message by itself also makes no sense without a stacktrace and source code. In fact, in most cases, a stacktrace and source code is exactly what a programmer needs to fix a bug, the error message doesn't help. If the precondition is not obvious, a comment next to it is a perfect way to document it.

The need to always supply an error message in cases no one really cares about the text leads to programmers making useless, obvious and not helpful messages which only makes the code less readable.

There are some cases, when an error message can be useful, for instance the message can contain the actual values of variables, but from our experience that is not very often, and in these cases you can use the message.

Similar arguments apply also in the assert methods in JUnit; always providing an error message is not a good idea.

Also, when you are using preconditions in a library rather than an application for end-users, things might be different since you don't know how your code will be used.

Theodor

Answer 5

As polygenelubricants pointed out:

For precondition checks, stating the requirement in the exception detail message is more informative than simply stating the facts.

While you can certainly do this with Guava Preconditions, Requirements API (which I authored) allows you to achieve better output without any effort on your part.

Guava Preconditions

String a = "foosball";
String b = "ballroom";
Preconditions.checkArgument(a == b, "a == b");

gives you this:

java.lang.IllegalArgumentException: a == b
    at com.google.common.base.Preconditions.checkArgument(Preconditions.java:122)
    at Main.main(Main.java:142)

Requirements API

String a = "foosball";
String b = "ballroom";
requireThat(a, "a").isEqualTo(b, "b")

gives you this:

Same input, better output.

(If your terminal does not support colors, you will get a textual diff instead)

Dacia