On 10 Nov 2015, at 18:55, Roger Riggs <roger.ri...@oracle.com> wrote:
> A few of the proposed replacements of ?: with requireNonNullElse were > unsuitable > because in the particular context, null is an allowed replacement value. > > The webrev has been updated to revert changes: > - two uses in > jdk/src/java.base/share/classes/java/time/format/DateTimePrintContext.java > (Line 145 and 160) > - src/java.base/share/classes/java/time/temporal/TemporalQueries.java - > reverted all changes > > webrev: > http://cr.openjdk.java.net/~rriggs/webrev-require-non-null-8141652/ Overall looks good. I’m not sure it is worth adding an instance field, zeroAliases, to Charset? -Chris. > > Thanks, Roger > > p.s. the previous webrev is renamed: > http://cr.openjdk.java.net/~rriggs/webrev-require-non-null-8141652-v1/ > > > > On 11/9/2015 2:35 PM, Roger Riggs wrote: >> 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 >>>> >> >
