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
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 >
