Hi Stephen,

On 11/6/2015 10:42 PM, Stephen Colebourne wrote:
Seems fine to me.

I would have inlined the ZoneId change to one line:
String id = Objects.requireNonNullElse(aliasMap.get(zoneId), zoneId);
to avoid the local variable change, but no big deal.
Stephen

Thanks, I'll fix that.

Roger



On 6 November 2015 at 20:24, Roger Riggs <roger.ri...@oracle.com> wrote:
Please review the renaming of java.util.Object methods, the new method names
are:
     public static <T> T requireNonNullElse(T obj, T defaultObj);
     public static <T> T requireNonNullElseGet(T obj, Supplier<? extends T>
supplier);

The only remaining possible gotcha for developers is the requireNonNull(T,
String)
method that might be confused with requireNonNullElse(T, T).  The longer
name is probably the one you want.
If you see a value that looks like an exception string, you've got the wrong
method.

To validate some of the uses, I visually scanned src/java.base looking
expressions of the pattern   (e1 != null) ? e1 : e2
Most uses of the ternary operator involve either primitive types or
conditions other than != null;
quite a few involved returning null and so were not appropriate to be
converted.

10 files contained relevant expressions:
  - 2 should be using Objects.toString() to provide a default value for a
String.
  - 2 can use the requireNonNullElseGet form to delay evaluating the
alternative value.
      (2 others were too early in the boot cycle to use lambda).
  - The rest followed the expected pattern; though checking if throwing an
exception was extra work.

Please review and comment.

Webrev:
http://cr.openjdk.java.net/~rriggs/webrev-require-non-null-8141652/

Thanks for the detailed evaluation and discussion.

Roger


Reply via email to