On 01/21/2015 12:01 PM, Paul Sandoz wrote:
On Jan 21, 2015, at 10:25 AM, Paul Sandoz <paul.san...@oracle.com> wrote:
Hi Andreas,
On Jan 21, 2015, at 10:00 AM, Andreas Lundblad <andreas.lundb...@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
