I really love how guava library allows simple one-liners for checking for null:
public void methodWithNullCheck(String couldBeNull) {
String definitelyNotNull = checkNotNull(couldBeNull);
//...
}
sadly, for simple argument check you need at least two lines of code:
public void methodWithArgCheck(String couldBeEmpty) {
checkArgument(!couldBeEmpty.isEmpty());
String definitelyNotEmpty = couldBeEmpty;
//...
}
however it is possible to add method which could do argument check and return a value if check successful. Below is an example of check and how it could be implemented:
public void methodWithEnhancedArgCheck(String couldBeEmpty) {
String definitelyNotEmpty = EnhancedPreconditions.checkArgument(couldBeEmpty, !couldBeEmpty.isEmpty());
//...
}
static class EnhancedPreconditions {
public static <T> T checkArgument(T reference, boolean expression) {
if (!expression) {
throw new IllegalArgumentException();
}
return reference;
}
}
I just was wondering is that by design and if it is worth to put feature request for that.
EDIT: @Nizet, yeah, checks in methods could be clumsy. However checks in constructors for nulls looks really good and saves a lot of time spent on debugging NPEs:
public class SomeClassWithDependency {
private final SomeDependency someDependency;
public SomeClassWithDependency(SomeDependency someDependency) {
this.someDependency = checkNotNull(someDependency);
}
//...
EDIT: Accepting Nizet's answer because I agree with him on side-effects and consistency reasoning. Also if you take a look into Xaerxess comment it looks like that causing confusion amongst other developers as well.
The biggest single reason that checkNotNull
returns its argument is so it can be used in constructors, like so:
public Foo(Bar bar) {
this.bar = checkNotNull(bar);
}
But the main reason that checkArgument
doesn't do something similar is that you'd have to pass the argument separately anyway, and it just doesn't seem worth it -- especially with more complicated precondition checks, which can sometimes be more readable on their own line. Just because something can be a one-liner doesn't mean that it should be, if it doesn't increase readability.
What I've never understood is why checkNotNull()
returns its argument in the first place:
public void foo(String bar) {
Preconditions.checkNotNull(bar);
// here, you're sure that bar is not null.
// No need to use another variable or to reassign bar to the result
// of checkNotNull()
}
I personally ignore the result of checkNotNull()
, as above. And this makes things consistent with the other checks which return void.
The only advantage I see is that you can do something like that, but I find it less readable than doing it in two separate lines:
public String trim(String bar) {
return Preconditions.checkNotNull(bar).trim();
}
So, in short, I agree with you that the API is somewhat inconsistent, but I would prefer for all the methods to return void. A method should either have a side effect, or return something, but doing both should be avoided generally. Here, the goal of the method is to have a side effect: throwing an exception.
EDIT:
Your example is indeed a more valid explanation of why returning the argument is useful. But I would still have favored consistency and cleanness instead of this possibility of checking and assigning in a single line.
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With