8050820: Please add java.util.Optional.stream() to convert Optional<T> to Stream<T>

Remi Forax forax at univ-mlv.fr
Wed Jan 21 11:12:06 UTC 2015


On 01/21/2015 12:01 PM, Paul Sandoz wrote:
> On Jan 21, 2015, at 10:25 AM, Paul Sandoz <Paul.Sandoz at oracle.com> wrote:
>
>> Hi Andreas,
>>
>> On Jan 21, 2015, at 10:00 AM, Andreas Lundblad <andreas.lundblad at oracle.com> wrote:
>>> Hi Paul,
>>>
>>> My name is Andreas Lundblad, and I joined the langtools team a little more than a year ago. I'm not a Reviewer or anything but I casually read through your patch out of curiosity.
>>>
>>> You don't seem to be a fan of the ?: operator. I would have written
>>>
>>>    return isPresent() ? Stream.of(value) : Stream.empty();
>>>
>>> Any reason for if/else in this case? Also, I find it strange to include { ... } only on the else-branch:
>>>
>>> +        if (!isPresent())
>>> +            return Stream.empty();
>>> +        else {
>>> +            return Stream.of(value);
>>> +        }
>>>
>>> It's in all of the if-statements in the patch so perhaps it's intentional. I think it looks a bit peculiar.
>>>
>> I copied the same style used in the existing methods.
>>
>> I personally would have used the ternary operator if i wrote this class :-) You are right, the inconsistent use of braces is odd, i will fix throughout to be consistent but will resist the urge to use the ternary operator.
>>
>> Even though you are not an official reviewer i can still add you as a reviewer of this patch.
>>
> I updated the webrev in place to be more consistent in the use of braces and better consistency for the primitive specializations:
>
>    http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8050820-Optional-stream/webrev/
>
> I don't wanna make any more syntax-related changes unless i done something silly.
>
> Paul.

This patch is good for me.
I'm glad to see that feature integrated into OptionalX.

Rémi




More information about the core-libs-dev mailing list