8141652 : Rename methods Objects.nonNullElse* to requireNonNullElse*
Roger Riggs
roger.riggs at oracle.com
Wed Nov 11 14:01:56 UTC 2015
Hi Chris,
Thanks for the review.
On 11/11/15 6:17 AM, Chris Hegarty wrote:
> On 10 Nov 2015, at 18:55, Roger Riggs <Roger.Riggs at 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?
As for the aliases array, the impact seemed smaller to create a constant
empty String[]
than to trigger the lambda mechanisms and be creating a new empty
String[] every time.
Plan B would probably be to revert to the previous code.
Thanks, Roger
>
> -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.Riggs at 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
>>>>>
More information about the core-libs-dev
mailing list