Hello,
On 04/10/2013 05:01 AM, Alan Bateman wrote:
On 09/04/2013 22:12, Joe Darcy wrote:
Hello,
Please review my changes for
8011800: Add java.util.Objects.requireNonNull(T, Supplier<String>)
http://cr.openjdk.java.net/~darcy/8011800.0/
which add a new method to java.util.Objects to take a
Supplier<String> rather than a String.
Patch inline below.
Thanks,
-Joe
A typo in the javadoc "this methods allows" -> "this method allows".
A subjective comment, but I would drop the word "sibling" from the
statement.
A minor nit with the @param spilling over into a second line is that
it might be clearer to indent it so that it's clear where the next tag
starts. I see the existing requiresNonNull are inconsistent on this
point.
The uninteresting Supplier is null case isn't specified, perhaps this
is deliberate?
That is deliberate. Instead of a body like
if (obj == null)
throw new NullPointerException(messageSupplier.get());
return obj;
I briefly considered something more elaborate like
if (obj == null)
throw new
NullPointerException(requireNonNull(messageSupplier.get(), "snarky
comment"));
return obj;
but I figured if you pass in a null message supplier, you get what you
deserve and it wasn't worth the extra cost of bloating the method body
and interfering with inlining.
A typo in the test at line 208, "rvariant" -> "variant".
Also the printed message at line 226 when you get don't pantaloons
should say "Supplier" rather than "2-arg".
I've reworded the specification:
/**
* Checks that the specified object reference is not {@code null} and
* throws a customized {@link NullPointerException} if it is.
*
* <p>Unlike the method {@link requireNonNull(Object, String},
* this method allows creation of the message to be deferred until
* after the null check is made. While this may confer a
* performance advantage in the non-null case, when deciding to
* call this method care should be taken that the costs of
* creating the message supplier are less than the cost of just
* creating the string message directly.
*
* @param obj the object reference to check for nullity
* @param messageSupplier supplier of the detail message to be
* used in the event that a {@code NullPointerException} is thrown
* @param <T> the type of the reference
* @return {@code obj} if not {@code null}
* @throws NullPointerException if {@code obj} is {@code null}
* @since 1.8
*/
and cleaned up the regression tests to use lambda expressions. New
webrev at
http://cr.openjdk.java.net/~darcy/8011800.1/
Thanks,
-Joe
