When doing null checks in Java code, and you throw IllegalArgumentExceptions for null values, what kind of message template do you use?
We tend to use something like this
public User getUser(String username){
if (username == null){
throw new IllegalArgumentException("username is null");
}
// ...
}
What is better : "is null" or "was null", and why?
For me "is null" feels more natural.
Since the Exception is thrown due to a failed precondition check, I think rather than simply stating a fact, you should state the requirement that was violated.
That is, instead of saying "username is null", say "username should not be null".
As a tip, you can use one of the many libraries designed to facilitate precondition checks. Many code in Guava uses com.google.common.base.Preconditions
Simple static methods to be called at the start of your own methods to verify correct arguments and state. 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);
More directly relevant here is that it has checkNotNull, which allows you to simply write:
checkNotNull(username, "username should not be null");
Note how naturally the above code reads, with the detailed message explicitly stating the requirement that was violated.
The alternative of stating facts is more awkward:
// Awkward!
checkArgument(count > 0, "is negative or zero: %s", count);
checkNotNull(username, "username is null");
Moreover, this is also potentially less useful, since the client may already be aware of the fact, and the exception doesn't help them figure out what the actual requirements are.
IllegalArgumentException vs NullPointerExceptionWhile your original code throws IllegalArgumentException on null arguments, Guava's Preconditions.checkNotNull throws NullPointerException instead.
This is in accordance with the guideline set by the API:
NullPointerException: Applications should throw instances of this class to indicate other illegal uses of thenullobject.
Additionally, here's a quote from Effective Java 2nd Edition: Item 60: Favor the use of standard exceptions:
Arguably, all erroneous method invocations boil down to an illegal argument or illegal state, but other exceptions are standardly used for certain kinds of illegal arguments and states. If a caller passes
nullin some parameter for which null values are prohibited, convention dictates thatNullPointerExceptionbe thrown rather thanIllegalArgumentException.
Preconditions.checkNotNull we had to choose one or the other, and went with NPE. One advantage of NPE is that if your method changes from not explicitly checking it (just dereferencing it) to explicitly checking, or vice versa, the exception type doesn't change. Some people believe that you should always thoroughly check everything even if the same exception would be thrown two lines down, but I see this as waste of time and space. –
Qatar is null, since the argument is still null..
However, why not simply throw a NullPointerException without a message?
I would suggest saying
if (userName == null) {
throw new IllegalArgumentException("username == null");
}
as this is so fatal that a programmer must look at it anyway. Referring the offending code snippet in the exception message is the concisest thing I can imagine.
I would be inclined to write this:
public User getUser(String username) {
if (username.length() == 0) {
throw new IllegalArgumentException("username is empty");
}
// ...
}
This kills two birds with one stone. First, it detects the case where user name is an empty string, which (for the sake of argument) I'm assuming is an error. Second, if the parameter is null attempting to dispatch the length call will give a NullPointerException.
For the record, the expected exception to throw for an unexpected null is NullPointerException. If your main reason for not using it is that NPE's typically don't have a message, code it like this:
public User getUser(String username){
if (username == null){
throw new NullPointerException("username is null");
}
if (username.length() == 0) {
throw new IllegalArgumentException("username is empty");
}
// ...
}
Why use NPE's here? Because NPE's almost always indicate a different kind of problem to a other kinds of argument validation error; e.g. a field or array cell that has not been initialized or an "optional" value that is not being handled properly.
Finally to the question:
What is better :
"is null"or"was null", and why?
This is a matter of opinion, but I would write "is null".
© 2022 - 2024 — McMap. All rights reserved.