Integrated: 8274394: Use Optional.isEmpty instead of !Optional.isPresent in jdk.jlink

2021-09-28 Thread Andrey Turbanov 
On Sun, 26 Sep 2021 13:58:35 GMT, Andrey Turbanov 
 wrote:

> I propose to replace usages of !Optional.isPresent() with Optional.isEmpty 
> method.
> It's makes code a bit easier to read.
> Noticing negation before long chain of method calls is hard.

This pull request has now been integrated.

Changeset: 6f4cefbc
Author:Andrey Turbanov 
Committer: Mandy Chung 
URL:   
https://git.openjdk.java.net/jdk/commit/6f4cefbcbaad38dcacd4e047c6c232a0a7a2c19c
Stats: 8 lines in 5 files changed: 0 ins; 0 del; 8 mod

8274394: Use Optional.isEmpty instead of !Optional.isPresent in jdk.jlink

Reviewed-by: alanb, mchung

-

PR: https://git.openjdk.java.net/jdk/pull/5707

Re: RFR: 8274394: Use Optional.isEmpty instead of !Optional.isPresent in jdk.jlink

2021-09-28 Thread Mandy Chung 
On Sun, 26 Sep 2021 13:58:35 GMT, Andrey Turbanov 
 wrote:

> I propose to replace usages of !Optional.isPresent() with Optional.isEmpty 
> method.
> It's makes code a bit easier to read.
> Noticing negation before long chain of method calls is hard.

LGTM

-

Marked as reviewed by mchung (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/5707

Re: RFR: 8274394: Use Optional.isEmpty instead of !Optional.isPresent in jdk.jlink

2021-09-28 Thread Alan Bateman 
On Sun, 26 Sep 2021 13:58:35 GMT, Andrey Turbanov 
 wrote:

> I propose to replace usages of !Optional.isPresent() with Optional.isEmpty 
> method.
> It's makes code a bit easier to read.
> Noticing negation before long chain of method calls is hard.

Looks okay, this code pre-dates the isEmpty method.

-

Marked as reviewed by alanb (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/5707

RFR: 8274394: Use Optional.isEmpty instead of !Optional.isPresent in jdk.jlink

2021-09-27 Thread Andrey Turbanov 
I propose to replace usages of !Optional.isPresent() with Optional.isEmpty 
method.
It's makes code a bit easier to read.
Noticing negation before long chain of method calls is hard.

-

Commit messages:
 - [PATCH] Use Optional.isEmpty instead of !Optional.isPresent in jdk.jlink

Changes: https://git.openjdk.java.net/jdk/pull/5707/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5707=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8274394
  Stats: 8 lines in 5 files changed: 0 ins; 0 del; 8 mod
  Patch: https://git.openjdk.java.net/jdk/pull/5707.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/5707/head:pull/5707

PR: https://git.openjdk.java.net/jdk/pull/5707

Re: RFR: 8184693: (opt) add Optional.isEmpty

2018-04-18 Thread Roger Riggs 

+1

There is one extra space in Optional.java: line 163:  "is__not".

Thanks, Roger


On 4/18/18 2:06 AM, Stuart Marks wrote:

OK, looks good! +1 from me.

s'marks

On 4/17/18 10:34 PM, Vivek Theeyarath wrote:

Hi Stuart,
Done with the changes 
http://cr.openjdk.java.net/~vtheeyarath/8184693/webrev.05/ .

Regards
Vivek
-Original Message-
From: Stuart Marks
Sent: Wednesday, April 18, 2018 8:56 AM
To: Vivek Theeyarath <vivek.theeyar...@oracle.com>
Cc: core-libs-dev <core-libs-dev@openjdk.java.net>; Paul Sandoz 
<paul.san...@oracle.com>

Subject: Re: RFR: 8184693: (opt) add Optional.isEmpty

Hi Vivek,

Thanks for the update. In the test files, please remove the 
unnecessary imports of List and the various Predicate types. In most 
cases it's not a problem to have unnecessary imports. I happened to 
notice in this case that they're left over from the previous version 
of the webrev, and looking at the current webrev by itself, it's 
clear they're unnecessary.


Thanks,

s'marks

On 4/17/18 6:23 AM, Vivek Theeyarath wrote:

Hi All,
Please find the updated webrev
http://cr.openjdk.java.net/~vtheeyarath/8184693/webrev.04
Regards
Vivek

-Original Message-
From: Stuart Marks
Sent: Tuesday, April 17, 2018 5:11 AM
To: Vivek Theeyarath <vivek.theeyar...@oracle.com>
Cc: core-libs-dev <core-libs-dev@openjdk.java.net>; Paul Sandoz
<paul.san...@oracle.com>
Subject: Re: RFR: 8184693: (opt) add Optional.isEmpty

Hi Vivek,

Please add "@since 11" tags to the doc comments of the four 
Optional*.isEmpty() methods.


Regarding the tests, I don't think the various newly added 
testIsEmpty() tests are useful. The setup in those test files 
already generates a fairly comprehensive set of Optional values from 
a variety of sources. All the assertion checking is performed in the 
checkEmpty() and checkPresent() methods.
It should be sufficient to add an assertTrue(isEmpty()) call to 
checkEmpty() -- as you've done -- and also to add an 
assertFalse(isEmpty()) call to checkPresent(). If these assertions 
are added, the additional testIsEmpty() test is unnecessary.


This applies to all four of the regression test files.

Thanks,

s'marks

On 4/16/18 11:08 AM, Vivek Theeyarath wrote:

Hi All,
Please find the updated webrev 
http://cr.openjdk.java.net/~vtheeyarath/8184693/webrev.02/ .

Here is the csr which I have raised for this change
https://bugs.openjdk.java.net/browse/JDK-8201606

Regards
Vivek
-Original Message-
From: Chris Hegarty
Sent: Sunday, April 15, 2018 6:48 PM
To: Vivek Theeyarath <vivek.theeyar...@oracle.com>
Cc: Remi Forax <fo...@univ-mlv.fr>; core-libs-dev
<core-libs-dev@openjdk.java.net>
Subject: Re: RFR: 8184693: (opt) add Optional.isEmpty



On 15 Apr 2018, at 11:25, Vivek Theeyarath 
<vivek.theeyar...@oracle.com> wrote:


Hi All,
  Please review
http://cr.openjdk.java.net/~vtheeyarath/8184693/webrev.01/


This looks ok to me.

For consistency, can you please update the copyright header year 
range in OptionalInt.


-Chris.


Regards
Vivek
-Original Message-
From: Vivek Theeyarath
Sent: Saturday, April 14, 2018 6:24 PM
To: Remi Forax <fo...@univ-mlv.fr>
Cc: core-libs-dev <core-libs-dev@openjdk.java.net>
Subject: RE: RFR: 8184693: (opt) add Optional.isEmpty

I missed that Remi. Thanks for pointing it out. Will address those 
and get back.


Regards
Vivek
-Original Message-
From: Remi Forax [mailto:fo...@univ-mlv.fr]
Sent: Saturday, April 14, 2018 2:58 PM
To: Vivek Theeyarath <vivek.theeyar...@oracle.com>
Cc: core-libs-dev <core-libs-dev@openjdk.java.net>
Subject: Re: RFR: 8184693: (opt) add Optional.isEmpty

Hi Vivek,
OptionalInt, OptionalLong and OptionalDouble should be changed too.

Rémi

- Mail original -

De: "Vivek Theeyarath" <vivek.theeyar...@oracle.com>
À: "core-libs-dev" <core-libs-dev@openjdk.java.net>
Envoyé: Samedi 14 Avril 2018 08:22:50
Objet: RFR: 8184693: (opt) add Optional.isEmpty



Hi All,

    Please review.

Bug: https://bugs.openjdk.java.net/browse/JDK-8184693

Webrev : http://cr.openjdk.java.net/~vtheeyarath/8184693/webrev.00/



The related jtreg test was run and the test passed .



Regards

Vivek

Re: RFR: 8184693: (opt) add Optional.isEmpty

2018-04-18 Thread Stuart Marks 

OK, looks good! +1 from me.

s'marks

On 4/17/18 10:34 PM, Vivek Theeyarath wrote:

Hi Stuart,
Done with the changes 
http://cr.openjdk.java.net/~vtheeyarath/8184693/webrev.05/ .
Regards
Vivek
-Original Message-
From: Stuart Marks
Sent: Wednesday, April 18, 2018 8:56 AM
To: Vivek Theeyarath <vivek.theeyar...@oracle.com>
Cc: core-libs-dev <core-libs-dev@openjdk.java.net>; Paul Sandoz 
<paul.san...@oracle.com>
Subject: Re: RFR: 8184693: (opt) add Optional.isEmpty

Hi Vivek,

Thanks for the update. In the test files, please remove the unnecessary imports 
of List and the various Predicate types. In most cases it's not a problem to 
have unnecessary imports. I happened to notice in this case that they're left 
over from the previous version of the webrev, and looking at the current webrev 
by itself, it's clear they're unnecessary.

Thanks,

s'marks

On 4/17/18 6:23 AM, Vivek Theeyarath wrote:

Hi All,
Please find the updated webrev
http://cr.openjdk.java.net/~vtheeyarath/8184693/webrev.04
Regards
Vivek

-Original Message-
From: Stuart Marks
Sent: Tuesday, April 17, 2018 5:11 AM
To: Vivek Theeyarath <vivek.theeyar...@oracle.com>
Cc: core-libs-dev <core-libs-dev@openjdk.java.net>; Paul Sandoz
<paul.san...@oracle.com>
Subject: Re: RFR: 8184693: (opt) add Optional.isEmpty

Hi Vivek,

Please add "@since 11" tags to the doc comments of the four Optional*.isEmpty() 
methods.

Regarding the tests, I don't think the various newly added testIsEmpty() tests 
are useful. The setup in those test files already generates a fairly 
comprehensive set of Optional values from a variety of sources. All the 
assertion checking is performed in the checkEmpty() and checkPresent() methods.
It should be sufficient to add an assertTrue(isEmpty()) call to checkEmpty() -- 
as you've done -- and also to add an assertFalse(isEmpty()) call to 
checkPresent(). If these assertions are added, the additional testIsEmpty() 
test is unnecessary.

This applies to all four of the regression test files.

Thanks,

s'marks

On 4/16/18 11:08 AM, Vivek Theeyarath wrote:

Hi All,
Please find the updated webrev 
http://cr.openjdk.java.net/~vtheeyarath/8184693/webrev.02/ .
Here is the csr which I have raised for this change
https://bugs.openjdk.java.net/browse/JDK-8201606

Regards
Vivek
-Original Message-
From: Chris Hegarty
Sent: Sunday, April 15, 2018 6:48 PM
To: Vivek Theeyarath <vivek.theeyar...@oracle.com>
Cc: Remi Forax <fo...@univ-mlv.fr>; core-libs-dev
<core-libs-dev@openjdk.java.net>
Subject: Re: RFR: 8184693: (opt) add Optional.isEmpty




On 15 Apr 2018, at 11:25, Vivek Theeyarath <vivek.theeyar...@oracle.com> wrote:

Hi All,
  Please review
http://cr.openjdk.java.net/~vtheeyarath/8184693/webrev.01/


This looks ok to me.

For consistency, can you please update the copyright header year range in 
OptionalInt.

-Chris.


Regards
Vivek
-Original Message-
From: Vivek Theeyarath
Sent: Saturday, April 14, 2018 6:24 PM
To: Remi Forax <fo...@univ-mlv.fr>
Cc: core-libs-dev <core-libs-dev@openjdk.java.net>
Subject: RE: RFR: 8184693: (opt) add Optional.isEmpty

I missed that Remi. Thanks for pointing it out. Will address those and get back.

Regards
Vivek
-Original Message-
From: Remi Forax [mailto:fo...@univ-mlv.fr]
Sent: Saturday, April 14, 2018 2:58 PM
To: Vivek Theeyarath <vivek.theeyar...@oracle.com>
Cc: core-libs-dev <core-libs-dev@openjdk.java.net>
Subject: Re: RFR: 8184693: (opt) add Optional.isEmpty

Hi Vivek,
OptionalInt, OptionalLong and OptionalDouble should be changed too.

Rémi

- Mail original -

De: "Vivek Theeyarath" <vivek.theeyar...@oracle.com>
À: "core-libs-dev" <core-libs-dev@openjdk.java.net>
Envoyé: Samedi 14 Avril 2018 08:22:50
Objet: RFR: 8184693: (opt) add Optional.isEmpty



Hi All,

Please review.

Bug: https://bugs.openjdk.java.net/browse/JDK-8184693

Webrev : http://cr.openjdk.java.net/~vtheeyarath/8184693/webrev.00/



The related jtreg test was run and the test passed .



Regards

Vivek

RE: RFR: 8184693: (opt) add Optional.isEmpty

2018-04-17 Thread Vivek Theeyarath 
Hi Stuart,
Done with the changes 
http://cr.openjdk.java.net/~vtheeyarath/8184693/webrev.05/ .
Regards
Vivek
-Original Message-
From: Stuart Marks 
Sent: Wednesday, April 18, 2018 8:56 AM
To: Vivek Theeyarath <vivek.theeyar...@oracle.com>
Cc: core-libs-dev <core-libs-dev@openjdk.java.net>; Paul Sandoz 
<paul.san...@oracle.com>
Subject: Re: RFR: 8184693: (opt) add Optional.isEmpty

Hi Vivek,

Thanks for the update. In the test files, please remove the unnecessary imports 
of List and the various Predicate types. In most cases it's not a problem to 
have unnecessary imports. I happened to notice in this case that they're left 
over from the previous version of the webrev, and looking at the current webrev 
by itself, it's clear they're unnecessary.

Thanks,

s'marks

On 4/17/18 6:23 AM, Vivek Theeyarath wrote:
> Hi All,
>   Please find the updated webrev 
> http://cr.openjdk.java.net/~vtheeyarath/8184693/webrev.04
> Regards
> Vivek
> 
> -Original Message-
> From: Stuart Marks
> Sent: Tuesday, April 17, 2018 5:11 AM
> To: Vivek Theeyarath <vivek.theeyar...@oracle.com>
> Cc: core-libs-dev <core-libs-dev@openjdk.java.net>; Paul Sandoz 
> <paul.san...@oracle.com>
> Subject: Re: RFR: 8184693: (opt) add Optional.isEmpty
> 
> Hi Vivek,
> 
> Please add "@since 11" tags to the doc comments of the four 
> Optional*.isEmpty() methods.
> 
> Regarding the tests, I don't think the various newly added testIsEmpty() 
> tests are useful. The setup in those test files already generates a fairly 
> comprehensive set of Optional values from a variety of sources. All the 
> assertion checking is performed in the checkEmpty() and checkPresent() 
> methods.
> It should be sufficient to add an assertTrue(isEmpty()) call to checkEmpty() 
> -- as you've done -- and also to add an assertFalse(isEmpty()) call to 
> checkPresent(). If these assertions are added, the additional testIsEmpty() 
> test is unnecessary.
> 
> This applies to all four of the regression test files.
> 
> Thanks,
> 
> s'marks
> 
> On 4/16/18 11:08 AM, Vivek Theeyarath wrote:
>> Hi All,
>>  Please find the updated webrev 
>> http://cr.openjdk.java.net/~vtheeyarath/8184693/webrev.02/ .
>> Here is the csr which I have raised for this change
>> https://bugs.openjdk.java.net/browse/JDK-8201606
>>
>> Regards
>> Vivek
>> -Original Message-
>> From: Chris Hegarty
>> Sent: Sunday, April 15, 2018 6:48 PM
>> To: Vivek Theeyarath <vivek.theeyar...@oracle.com>
>> Cc: Remi Forax <fo...@univ-mlv.fr>; core-libs-dev 
>> <core-libs-dev@openjdk.java.net>
>> Subject: Re: RFR: 8184693: (opt) add Optional.isEmpty
>>
>>
>>
>>> On 15 Apr 2018, at 11:25, Vivek Theeyarath <vivek.theeyar...@oracle.com> 
>>> wrote:
>>>
>>> Hi All,
>>>  Please review
>>> http://cr.openjdk.java.net/~vtheeyarath/8184693/webrev.01/
>>
>> This looks ok to me.
>>
>> For consistency, can you please update the copyright header year range in 
>> OptionalInt.
>>
>> -Chris.
>>
>>> Regards
>>> Vivek
>>> -Original Message-
>>> From: Vivek Theeyarath
>>> Sent: Saturday, April 14, 2018 6:24 PM
>>> To: Remi Forax <fo...@univ-mlv.fr>
>>> Cc: core-libs-dev <core-libs-dev@openjdk.java.net>
>>> Subject: RE: RFR: 8184693: (opt) add Optional.isEmpty
>>>
>>> I missed that Remi. Thanks for pointing it out. Will address those and get 
>>> back.
>>>
>>> Regards
>>> Vivek
>>> -Original Message-
>>> From: Remi Forax [mailto:fo...@univ-mlv.fr]
>>> Sent: Saturday, April 14, 2018 2:58 PM
>>> To: Vivek Theeyarath <vivek.theeyar...@oracle.com>
>>> Cc: core-libs-dev <core-libs-dev@openjdk.java.net>
>>> Subject: Re: RFR: 8184693: (opt) add Optional.isEmpty
>>>
>>> Hi Vivek,
>>> OptionalInt, OptionalLong and OptionalDouble should be changed too.
>>>
>>> Rémi
>>>
>>> - Mail original -
>>>> De: "Vivek Theeyarath" <vivek.theeyar...@oracle.com>
>>>> À: "core-libs-dev" <core-libs-dev@openjdk.java.net>
>>>> Envoyé: Samedi 14 Avril 2018 08:22:50
>>>> Objet: RFR: 8184693: (opt) add Optional.isEmpty
>>>
>>>> Hi All,
>>>>
>>>>Please review.
>>>>
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8184693
>>>>
>>>> Webrev : http://cr.openjdk.java.net/~vtheeyarath/8184693/webrev.00/
>>>>
>>>>
>>>>
>>>> The related jtreg test was run and the test passed .
>>>>
>>>>
>>>>
>>>> Regards
>>>>
>>>> Vivek
>>

Re: RFR: 8184693: (opt) add Optional.isEmpty

2018-04-17 Thread Stuart Marks 

Hi Vivek,

Thanks for the update. In the test files, please remove the unnecessary imports 
of List and the various Predicate types. In most cases it's not a problem to 
have unnecessary imports. I happened to notice in this case that they're left 
over from the previous version of the webrev, and looking at the current webrev 
by itself, it's clear they're unnecessary.


Thanks,

s'marks

On 4/17/18 6:23 AM, Vivek Theeyarath wrote:

Hi All,
Please find the updated webrev 
http://cr.openjdk.java.net/~vtheeyarath/8184693/webrev.04
Regards
Vivek

-Original Message-
From: Stuart Marks
Sent: Tuesday, April 17, 2018 5:11 AM
To: Vivek Theeyarath <vivek.theeyar...@oracle.com>
Cc: core-libs-dev <core-libs-dev@openjdk.java.net>; Paul Sandoz 
<paul.san...@oracle.com>
Subject: Re: RFR: 8184693: (opt) add Optional.isEmpty

Hi Vivek,

Please add "@since 11" tags to the doc comments of the four Optional*.isEmpty() 
methods.

Regarding the tests, I don't think the various newly added testIsEmpty() tests 
are useful. The setup in those test files already generates a fairly 
comprehensive set of Optional values from a variety of sources. All the 
assertion checking is performed in the checkEmpty() and checkPresent() methods.
It should be sufficient to add an assertTrue(isEmpty()) call to checkEmpty() -- 
as you've done -- and also to add an assertFalse(isEmpty()) call to 
checkPresent(). If these assertions are added, the additional testIsEmpty() 
test is unnecessary.

This applies to all four of the regression test files.

Thanks,

s'marks

On 4/16/18 11:08 AM, Vivek Theeyarath wrote:

Hi All,
Please find the updated webrev 
http://cr.openjdk.java.net/~vtheeyarath/8184693/webrev.02/ .
Here is the csr which I have raised for this change
https://bugs.openjdk.java.net/browse/JDK-8201606

Regards
Vivek
-Original Message-
From: Chris Hegarty
Sent: Sunday, April 15, 2018 6:48 PM
To: Vivek Theeyarath <vivek.theeyar...@oracle.com>
Cc: Remi Forax <fo...@univ-mlv.fr>; core-libs-dev
<core-libs-dev@openjdk.java.net>
Subject: Re: RFR: 8184693: (opt) add Optional.isEmpty




On 15 Apr 2018, at 11:25, Vivek Theeyarath <vivek.theeyar...@oracle.com> wrote:

Hi All,
 Please review
http://cr.openjdk.java.net/~vtheeyarath/8184693/webrev.01/


This looks ok to me.

For consistency, can you please update the copyright header year range in 
OptionalInt.

-Chris.


Regards
Vivek
-Original Message-
From: Vivek Theeyarath
Sent: Saturday, April 14, 2018 6:24 PM
To: Remi Forax <fo...@univ-mlv.fr>
Cc: core-libs-dev <core-libs-dev@openjdk.java.net>
Subject: RE: RFR: 8184693: (opt) add Optional.isEmpty

I missed that Remi. Thanks for pointing it out. Will address those and get back.

Regards
Vivek
-Original Message-
From: Remi Forax [mailto:fo...@univ-mlv.fr]
Sent: Saturday, April 14, 2018 2:58 PM
To: Vivek Theeyarath <vivek.theeyar...@oracle.com>
Cc: core-libs-dev <core-libs-dev@openjdk.java.net>
Subject: Re: RFR: 8184693: (opt) add Optional.isEmpty

Hi Vivek,
OptionalInt, OptionalLong and OptionalDouble should be changed too.

Rémi

- Mail original -

De: "Vivek Theeyarath" <vivek.theeyar...@oracle.com>
À: "core-libs-dev" <core-libs-dev@openjdk.java.net>
Envoyé: Samedi 14 Avril 2018 08:22:50
Objet: RFR: 8184693: (opt) add Optional.isEmpty



Hi All,

   Please review.

Bug: https://bugs.openjdk.java.net/browse/JDK-8184693

Webrev : http://cr.openjdk.java.net/~vtheeyarath/8184693/webrev.00/



The related jtreg test was run and the test passed .



Regards

Vivek

RE: RFR: 8184693: (opt) add Optional.isEmpty

2018-04-17 Thread Vivek Theeyarath 
Hi All,
Please find the updated webrev 
http://cr.openjdk.java.net/~vtheeyarath/8184693/webrev.04 
Regards
Vivek

-Original Message-
From: Stuart Marks 
Sent: Tuesday, April 17, 2018 5:11 AM
To: Vivek Theeyarath <vivek.theeyar...@oracle.com>
Cc: core-libs-dev <core-libs-dev@openjdk.java.net>; Paul Sandoz 
<paul.san...@oracle.com>
Subject: Re: RFR: 8184693: (opt) add Optional.isEmpty

Hi Vivek,

Please add "@since 11" tags to the doc comments of the four Optional*.isEmpty() 
methods.

Regarding the tests, I don't think the various newly added testIsEmpty() tests 
are useful. The setup in those test files already generates a fairly 
comprehensive set of Optional values from a variety of sources. All the 
assertion checking is performed in the checkEmpty() and checkPresent() methods. 
It should be sufficient to add an assertTrue(isEmpty()) call to checkEmpty() -- 
as you've done -- and also to add an assertFalse(isEmpty()) call to 
checkPresent(). If these assertions are added, the additional testIsEmpty() 
test is unnecessary.

This applies to all four of the regression test files.

Thanks,

s'marks

On 4/16/18 11:08 AM, Vivek Theeyarath wrote:
> Hi All,
>   Please find the updated webrev 
> http://cr.openjdk.java.net/~vtheeyarath/8184693/webrev.02/ .
> Here is the csr which I have raised for this change 
> https://bugs.openjdk.java.net/browse/JDK-8201606
> 
> Regards
> Vivek
> -Original Message-
> From: Chris Hegarty
> Sent: Sunday, April 15, 2018 6:48 PM
> To: Vivek Theeyarath <vivek.theeyar...@oracle.com>
> Cc: Remi Forax <fo...@univ-mlv.fr>; core-libs-dev 
> <core-libs-dev@openjdk.java.net>
> Subject: Re: RFR: 8184693: (opt) add Optional.isEmpty
> 
> 
> 
>> On 15 Apr 2018, at 11:25, Vivek Theeyarath <vivek.theeyar...@oracle.com> 
>> wrote:
>>
>> Hi All,
>> Please review 
>> http://cr.openjdk.java.net/~vtheeyarath/8184693/webrev.01/
> 
> This looks ok to me.
> 
> For consistency, can you please update the copyright header year range in 
> OptionalInt.
> 
> -Chris.
> 
>> Regards
>> Vivek
>> -Original Message-
>> From: Vivek Theeyarath
>> Sent: Saturday, April 14, 2018 6:24 PM
>> To: Remi Forax <fo...@univ-mlv.fr>
>> Cc: core-libs-dev <core-libs-dev@openjdk.java.net>
>> Subject: RE: RFR: 8184693: (opt) add Optional.isEmpty
>>
>> I missed that Remi. Thanks for pointing it out. Will address those and get 
>> back.
>>
>> Regards
>> Vivek
>> -Original Message-
>> From: Remi Forax [mailto:fo...@univ-mlv.fr]
>> Sent: Saturday, April 14, 2018 2:58 PM
>> To: Vivek Theeyarath <vivek.theeyar...@oracle.com>
>> Cc: core-libs-dev <core-libs-dev@openjdk.java.net>
>> Subject: Re: RFR: 8184693: (opt) add Optional.isEmpty
>>
>> Hi Vivek,
>> OptionalInt, OptionalLong and OptionalDouble should be changed too.
>>
>> Rémi
>>
>> - Mail original -
>>> De: "Vivek Theeyarath" <vivek.theeyar...@oracle.com>
>>> À: "core-libs-dev" <core-libs-dev@openjdk.java.net>
>>> Envoyé: Samedi 14 Avril 2018 08:22:50
>>> Objet: RFR: 8184693: (opt) add Optional.isEmpty
>>
>>> Hi All,
>>>
>>>   Please review.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8184693
>>>
>>> Webrev : http://cr.openjdk.java.net/~vtheeyarath/8184693/webrev.00/
>>>
>>>
>>>
>>> The related jtreg test was run and the test passed .
>>>
>>>
>>>
>>> Regards
>>>
>>> Vivek
>

Re: RFR: 8184693: (opt) add Optional.isEmpty

2018-04-16 Thread Stuart Marks 

Hi Vivek,

Please add "@since 11" tags to the doc comments of the four Optional*.isEmpty() 
methods.


Regarding the tests, I don't think the various newly added testIsEmpty() tests 
are useful. The setup in those test files already generates a fairly 
comprehensive set of Optional values from a variety of sources. All the 
assertion checking is performed in the checkEmpty() and checkPresent() methods. 
It should be sufficient to add an assertTrue(isEmpty()) call to checkEmpty() -- 
as you've done -- and also to add an assertFalse(isEmpty()) call to 
checkPresent(). If these assertions are added, the additional testIsEmpty() test 
is unnecessary.


This applies to all four of the regression test files.

Thanks,

s'marks

On 4/16/18 11:08 AM, Vivek Theeyarath wrote:

Hi All,
Please find the updated webrev 
http://cr.openjdk.java.net/~vtheeyarath/8184693/webrev.02/ .
Here is the csr which I have raised for this change 
https://bugs.openjdk.java.net/browse/JDK-8201606

Regards
Vivek
-Original Message-
From: Chris Hegarty
Sent: Sunday, April 15, 2018 6:48 PM
To: Vivek Theeyarath <vivek.theeyar...@oracle.com>
Cc: Remi Forax <fo...@univ-mlv.fr>; core-libs-dev 
<core-libs-dev@openjdk.java.net>
Subject: Re: RFR: 8184693: (opt) add Optional.isEmpty




On 15 Apr 2018, at 11:25, Vivek Theeyarath <vivek.theeyar...@oracle.com> wrote:

Hi All,
Please review http://cr.openjdk.java.net/~vtheeyarath/8184693/webrev.01/


This looks ok to me.

For consistency, can you please update the copyright header year range in 
OptionalInt.

-Chris.


Regards
Vivek
-Original Message-
From: Vivek Theeyarath
Sent: Saturday, April 14, 2018 6:24 PM
To: Remi Forax <fo...@univ-mlv.fr>
Cc: core-libs-dev <core-libs-dev@openjdk.java.net>
Subject: RE: RFR: 8184693: (opt) add Optional.isEmpty

I missed that Remi. Thanks for pointing it out. Will address those and get back.

Regards
Vivek
-Original Message-
From: Remi Forax [mailto:fo...@univ-mlv.fr]
Sent: Saturday, April 14, 2018 2:58 PM
To: Vivek Theeyarath <vivek.theeyar...@oracle.com>
Cc: core-libs-dev <core-libs-dev@openjdk.java.net>
Subject: Re: RFR: 8184693: (opt) add Optional.isEmpty

Hi Vivek,
OptionalInt, OptionalLong and OptionalDouble should be changed too.

Rémi

- Mail original -

De: "Vivek Theeyarath" <vivek.theeyar...@oracle.com>
À: "core-libs-dev" <core-libs-dev@openjdk.java.net>
Envoyé: Samedi 14 Avril 2018 08:22:50
Objet: RFR: 8184693: (opt) add Optional.isEmpty



Hi All,

  Please review.

Bug: https://bugs.openjdk.java.net/browse/JDK-8184693

Webrev : http://cr.openjdk.java.net/~vtheeyarath/8184693/webrev.00/



The related jtreg test was run and the test passed .



Regards

Vivek

RE: RFR: 8184693: (opt) add Optional.isEmpty

2018-04-16 Thread Vivek Theeyarath 
Hi All,
Please find the updated webrev 
http://cr.openjdk.java.net/~vtheeyarath/8184693/webrev.02/ .
Here is the csr which I have raised for this change 
https://bugs.openjdk.java.net/browse/JDK-8201606 

Regards
Vivek
-Original Message-
From: Chris Hegarty 
Sent: Sunday, April 15, 2018 6:48 PM
To: Vivek Theeyarath <vivek.theeyar...@oracle.com>
Cc: Remi Forax <fo...@univ-mlv.fr>; core-libs-dev 
<core-libs-dev@openjdk.java.net>
Subject: Re: RFR: 8184693: (opt) add Optional.isEmpty



> On 15 Apr 2018, at 11:25, Vivek Theeyarath <vivek.theeyar...@oracle.com> 
> wrote:
> 
> Hi All,
>Please review http://cr.openjdk.java.net/~vtheeyarath/8184693/webrev.01/ 

This looks ok to me.

For consistency, can you please update the copyright header year range in 
OptionalInt.

-Chris.

> Regards
> Vivek
> -Original Message-
> From: Vivek Theeyarath 
> Sent: Saturday, April 14, 2018 6:24 PM
> To: Remi Forax <fo...@univ-mlv.fr>
> Cc: core-libs-dev <core-libs-dev@openjdk.java.net>
> Subject: RE: RFR: 8184693: (opt) add Optional.isEmpty
> 
> I missed that Remi. Thanks for pointing it out. Will address those and get 
> back.
> 
> Regards
> Vivek
> -Original Message-
> From: Remi Forax [mailto:fo...@univ-mlv.fr] 
> Sent: Saturday, April 14, 2018 2:58 PM
> To: Vivek Theeyarath <vivek.theeyar...@oracle.com>
> Cc: core-libs-dev <core-libs-dev@openjdk.java.net>
> Subject: Re: RFR: 8184693: (opt) add Optional.isEmpty
> 
> Hi Vivek,
> OptionalInt, OptionalLong and OptionalDouble should be changed too.
> 
> Rémi
> 
> - Mail original -
>> De: "Vivek Theeyarath" <vivek.theeyar...@oracle.com>
>> À: "core-libs-dev" <core-libs-dev@openjdk.java.net>
>> Envoyé: Samedi 14 Avril 2018 08:22:50
>> Objet: RFR: 8184693: (opt) add Optional.isEmpty
> 
>> Hi All,
>> 
>>  Please review.
>> 
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8184693
>> 
>> Webrev : http://cr.openjdk.java.net/~vtheeyarath/8184693/webrev.00/
>> 
>> 
>> 
>> The related jtreg test was run and the test passed .
>> 
>> 
>> 
>> Regards
>> 
>> Vivek

Re: RFR: 8184693: (opt) add Optional.isEmpty

2018-04-15 Thread Chris Hegarty 


> On 15 Apr 2018, at 11:25, Vivek Theeyarath <vivek.theeyar...@oracle.com> 
> wrote:
> 
> Hi All,
>Please review http://cr.openjdk.java.net/~vtheeyarath/8184693/webrev.01/ 

This looks ok to me.

For consistency, can you please update the copyright header year range in 
OptionalInt.

-Chris.

> Regards
> Vivek
> -Original Message-
> From: Vivek Theeyarath 
> Sent: Saturday, April 14, 2018 6:24 PM
> To: Remi Forax <fo...@univ-mlv.fr>
> Cc: core-libs-dev <core-libs-dev@openjdk.java.net>
> Subject: RE: RFR: 8184693: (opt) add Optional.isEmpty
> 
> I missed that Remi. Thanks for pointing it out. Will address those and get 
> back.
> 
> Regards
> Vivek
> -Original Message-
> From: Remi Forax [mailto:fo...@univ-mlv.fr] 
> Sent: Saturday, April 14, 2018 2:58 PM
> To: Vivek Theeyarath <vivek.theeyar...@oracle.com>
> Cc: core-libs-dev <core-libs-dev@openjdk.java.net>
> Subject: Re: RFR: 8184693: (opt) add Optional.isEmpty
> 
> Hi Vivek,
> OptionalInt, OptionalLong and OptionalDouble should be changed too.
> 
> Rémi
> 
> - Mail original -
>> De: "Vivek Theeyarath" <vivek.theeyar...@oracle.com>
>> À: "core-libs-dev" <core-libs-dev@openjdk.java.net>
>> Envoyé: Samedi 14 Avril 2018 08:22:50
>> Objet: RFR: 8184693: (opt) add Optional.isEmpty
> 
>> Hi All,
>> 
>>  Please review.
>> 
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8184693
>> 
>> Webrev : http://cr.openjdk.java.net/~vtheeyarath/8184693/webrev.00/
>> 
>> 
>> 
>> The related jtreg test was run and the test passed .
>> 
>> 
>> 
>> Regards
>> 
>> Vivek

Re: RFR: 8184693: (opt) add Optional.isEmpty

2018-04-15 Thread forax 
Hi Vivek,

Looks good to me,
nitpicking, when creating the list with List.of() in the tests, a space after 
each comma is missing 
  var list = List.of(2, 3, 4);

cheers,
Rémi

- Mail original -
> De: "Vivek Theeyarath" <vivek.theeyar...@oracle.com>
> À: "Remi Forax" <fo...@univ-mlv.fr>, "core-libs-dev" 
> <core-libs-dev@openjdk.java.net>
> Envoyé: Dimanche 15 Avril 2018 12:25:09
> Objet: RE: RFR: 8184693: (opt) add Optional.isEmpty

> Hi All,
>   Please review http://cr.openjdk.java.net/~vtheeyarath/8184693/webrev.01/
> 
> Regards
> Vivek
> -Original Message-
> From: Vivek Theeyarath
> Sent: Saturday, April 14, 2018 6:24 PM
> To: Remi Forax <fo...@univ-mlv.fr>
> Cc: core-libs-dev <core-libs-dev@openjdk.java.net>
> Subject: RE: RFR: 8184693: (opt) add Optional.isEmpty
> 
> I missed that Remi. Thanks for pointing it out. Will address those and get 
> back.
> 
> Regards
> Vivek
> -Original Message-
> From: Remi Forax [mailto:fo...@univ-mlv.fr]
> Sent: Saturday, April 14, 2018 2:58 PM
> To: Vivek Theeyarath <vivek.theeyar...@oracle.com>
> Cc: core-libs-dev <core-libs-dev@openjdk.java.net>
> Subject: Re: RFR: 8184693: (opt) add Optional.isEmpty
> 
> Hi Vivek,
> OptionalInt, OptionalLong and OptionalDouble should be changed too.
> 
> Rémi
> 
> - Mail original -
>> De: "Vivek Theeyarath" <vivek.theeyar...@oracle.com>
>> À: "core-libs-dev" <core-libs-dev@openjdk.java.net>
>> Envoyé: Samedi 14 Avril 2018 08:22:50
>> Objet: RFR: 8184693: (opt) add Optional.isEmpty
> 
>> Hi All,
>> 
>>   Please review.
>> 
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8184693
>> 
>> Webrev : http://cr.openjdk.java.net/~vtheeyarath/8184693/webrev.00/
>> 
>> 
>> 
>> The related jtreg test was run and the test passed .
>> 
>> 
>> 
>> Regards
>> 
> > Vivek

RE: RFR: 8184693: (opt) add Optional.isEmpty

2018-04-15 Thread Vivek Theeyarath 
Hi All,
Please review 
http://cr.openjdk.java.net/~vtheeyarath/8184693/webrev.01/ 

Regards
Vivek
-Original Message-
From: Vivek Theeyarath 
Sent: Saturday, April 14, 2018 6:24 PM
To: Remi Forax <fo...@univ-mlv.fr>
Cc: core-libs-dev <core-libs-dev@openjdk.java.net>
Subject: RE: RFR: 8184693: (opt) add Optional.isEmpty

I missed that Remi. Thanks for pointing it out. Will address those and get back.

Regards
Vivek
-Original Message-
From: Remi Forax [mailto:fo...@univ-mlv.fr] 
Sent: Saturday, April 14, 2018 2:58 PM
To: Vivek Theeyarath <vivek.theeyar...@oracle.com>
Cc: core-libs-dev <core-libs-dev@openjdk.java.net>
Subject: Re: RFR: 8184693: (opt) add Optional.isEmpty

Hi Vivek,
OptionalInt, OptionalLong and OptionalDouble should be changed too.

Rémi

- Mail original -
> De: "Vivek Theeyarath" <vivek.theeyar...@oracle.com>
> À: "core-libs-dev" <core-libs-dev@openjdk.java.net>
> Envoyé: Samedi 14 Avril 2018 08:22:50
> Objet: RFR: 8184693: (opt) add Optional.isEmpty

> Hi All,
> 
>   Please review.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8184693
> 
> Webrev : http://cr.openjdk.java.net/~vtheeyarath/8184693/webrev.00/
> 
> 
> 
> The related jtreg test was run and the test passed .
> 
> 
> 
> Regards
> 
> Vivek

RE: RFR: 8184693: (opt) add Optional.isEmpty

2018-04-14 Thread Vivek Theeyarath 
I missed that Remi. Thanks for pointing it out. Will address those and get back.

Regards
Vivek
-Original Message-
From: Remi Forax [mailto:fo...@univ-mlv.fr] 
Sent: Saturday, April 14, 2018 2:58 PM
To: Vivek Theeyarath <vivek.theeyar...@oracle.com>
Cc: core-libs-dev <core-libs-dev@openjdk.java.net>
Subject: Re: RFR: 8184693: (opt) add Optional.isEmpty

Hi Vivek,
OptionalInt, OptionalLong and OptionalDouble should be changed too.

Rémi

- Mail original -
> De: "Vivek Theeyarath" <vivek.theeyar...@oracle.com>
> À: "core-libs-dev" <core-libs-dev@openjdk.java.net>
> Envoyé: Samedi 14 Avril 2018 08:22:50
> Objet: RFR: 8184693: (opt) add Optional.isEmpty

> Hi All,
> 
>   Please review.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8184693
> 
> Webrev : http://cr.openjdk.java.net/~vtheeyarath/8184693/webrev.00/
> 
> 
> 
> The related jtreg test was run and the test passed .
> 
> 
> 
> Regards
> 
> Vivek

Re: RFR: 8184693: (opt) add Optional.isEmpty

2018-04-14 Thread Remi Forax 
Hi Vivek,
the parenthesis around the lambdas in Basic.java are unnecessary
  Predicate isPositiveNumber =  (x -> x > 0);
vs
  Predicate isPositiveNumber =  x -> x > 0;

the same remark for isNegativeNumber.

cheers,
Rémi

- Mail original -
> De: "Vivek Theeyarath" <vivek.theeyar...@oracle.com>
> À: "core-libs-dev" <core-libs-dev@openjdk.java.net>
> Envoyé: Samedi 14 Avril 2018 08:22:50
> Objet: RFR: 8184693: (opt) add Optional.isEmpty

> Hi All,
> 
>   Please review.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8184693
> 
> Webrev : http://cr.openjdk.java.net/~vtheeyarath/8184693/webrev.00/
> 
> 
> 
> The related jtreg test was run and the test passed .
> 
> 
> 
> Regards
> 
> Vivek

Re: RFR: 8184693: (opt) add Optional.isEmpty

2018-04-14 Thread Remi Forax 
Hi Vivek,
OptionalInt, OptionalLong and OptionalDouble should be changed too.

Rémi

- Mail original -
> De: "Vivek Theeyarath" <vivek.theeyar...@oracle.com>
> À: "core-libs-dev" <core-libs-dev@openjdk.java.net>
> Envoyé: Samedi 14 Avril 2018 08:22:50
> Objet: RFR: 8184693: (opt) add Optional.isEmpty

> Hi All,
> 
>   Please review.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8184693
> 
> Webrev : http://cr.openjdk.java.net/~vtheeyarath/8184693/webrev.00/
> 
> 
> 
> The related jtreg test was run and the test passed .
> 
> 
> 
> Regards
> 
> Vivek

RFR: 8184693: (opt) add Optional.isEmpty

2018-04-14 Thread Vivek Theeyarath 
Hi All,

   Please review.

Bug: https://bugs.openjdk.java.net/browse/JDK-8184693 

Webrev : http://cr.openjdk.java.net/~vtheeyarath/8184693/webrev.00/ 

 

The related jtreg test was run and the test passed .

 

Regards

Vivek

Re: Optional.isEmpty()

2017-04-24 Thread Roger Riggs 

Hi,

IMHO,boolean isEmpty() would be a good complement to the existing 
empty() method.


$.02, Roger


On 4/24/2017 1:15 PM, Anthony Vanelverdinghe wrote:

Hi Peter

I'd say no: it's merely the negation of an existing method, and given 
that the bar for adding methods to Optional is set very high (see e.g. 
[1] and [2]), I don't see how this one would meet it.


Moreover, I don't see any issues with simply writing:

return !cf.findModule(target).isPresent();

and there are plenty of one-liners that don't use boolean negation as 
well, for example:


return cf.findModule(target).orElse(null) == null; // [3]
return cf.findModule(target).map(m -> false).orElse(true); // [4]
return cf.findModule(target).stream().count() == 0;
return cf.findModule(target).stream().allMatch(m -> false);
return cf.findModule(target).isPresent() ^ true;
...
return cf.findModule(target).equals(Optional.empty());

Adding a static import to the last one gets you very close to the 
proposed method already:

return cf.findModule(target).equals(empty());

Of course, another option is to introduce a variable to avoid the 
combination of boolean negation and method chaining:

boolean moduleFound = cf.findModule(target).isPresent();
return !moduleFound;

Kind regards, Anthony

[1] https://bugs.openjdk.java.net/browse/JDK-8057557
[2] https://bugs.openjdk.java.net/browse/JDK-8058707
[3] 
http://mail.openjdk.java.net/pipermail/jigsaw-dev/2017-April/012273.html
[4] 
http://mail.openjdk.java.net/pipermail/jigsaw-dev/2017-April/012290.html



On 22/04/2017 11:40, peter.levart at gmail.com (Peter Levart) wrote:

Hi,

Seeing the following line in some JDK test that was up for review:

  return cf.findModule(target).orElse(null) == null;

I immediately jumped to suggest it would look better if written as:

  return !cf.findModule(target).isPresent();


But then I leaned back and asked myself: "Would it really?"

The boolean negation in front of a chain of method calls tends to
visually disappear from the view when we finally reach the offending
method and might get missed when quickly browsing the code. In this
respect, ".orElse(null) == null" is actually better. But the best would
of course be something like that:

  return cf.findModule(target).isEmpty();

What do you think? Would this pull its weight?

Regards, Peter

Re: Optional.isEmpty()

2017-04-24 Thread Anthony Vanelverdinghe 

Hi Peter

I'd say no: it's merely the negation of an existing method, and given 
that the bar for adding methods to Optional is set very high (see e.g. 
[1] and [2]), I don't see how this one would meet it.


Moreover, I don't see any issues with simply writing:

return !cf.findModule(target).isPresent();

and there are plenty of one-liners that don't use boolean negation as 
well, for example:


return cf.findModule(target).orElse(null) == null; // [3]
return cf.findModule(target).map(m -> false).orElse(true); // [4]
return cf.findModule(target).stream().count() == 0;
return cf.findModule(target).stream().allMatch(m -> false);
return cf.findModule(target).isPresent() ^ true;
...
return cf.findModule(target).equals(Optional.empty());

Adding a static import to the last one gets you very close to the 
proposed method already:

return cf.findModule(target).equals(empty());

Of course, another option is to introduce a variable to avoid the 
combination of boolean negation and method chaining:

boolean moduleFound = cf.findModule(target).isPresent();
return !moduleFound;

Kind regards, Anthony

[1] https://bugs.openjdk.java.net/browse/JDK-8057557
[2] https://bugs.openjdk.java.net/browse/JDK-8058707
[3] http://mail.openjdk.java.net/pipermail/jigsaw-dev/2017-April/012273.html
[4] http://mail.openjdk.java.net/pipermail/jigsaw-dev/2017-April/012290.html


On 22/04/2017 11:40, peter.levart at gmail.com (Peter Levart) wrote:

Hi,

Seeing the following line in some JDK test that was up for review:

  return cf.findModule(target).orElse(null) == null;

I immediately jumped to suggest it would look better if written as:

  return !cf.findModule(target).isPresent();


But then I leaned back and asked myself: "Would it really?"

The boolean negation in front of a chain of method calls tends to
visually disappear from the view when we finally reach the offending
method and might get missed when quickly browsing the code. In this
respect, ".orElse(null) == null" is actually better. But the best would
of course be something like that:

  return cf.findModule(target).isEmpty();

What do you think? Would this pull its weight?

Regards, Peter

Re: Optional.isEmpty()

2017-04-24 Thread Sander Mak 

> On 22 Apr 2017, at 11:40, Peter Levart  wrote:
>return cf.findModule(target).isEmpty();
> 
> What do you think? Would this pull its weight?

If I had a nickel for each time I started typing .isEm.., I'd have a 
respectable nickel collection. Big +1 from me.


Sander

Re: Optional.isEmpty()

2017-04-24 Thread dalibor topic 



On 24.04.2017 10:26, Andrew Dinn wrote:

Ah, bike-shedding!

Personally, I much prefer isAbsent() to isNotPresent(), presence and
absence being a historically well-sanctioned English language pairing.

[n.b. I'll grant that my preference for C18th literature over Comp Sci
argot might have swayed my judgement in this matter so I'll leave it to
others to sanction or reject that suggestion via a conformantly aligned,
textually rendered act of pollification (i.e. go on, post yer +/-1
follow-ups)]


If we're going with fine aged literature references for naming things, 
then isBarren() might signify appealing, yet slightly terrifying 
existential emptiness. [0] [1]


cheers,
dalibor topic

[0] 'The Draculas were, says Arminius, a great and noble race, though 
now and again were scions who were held by their coevals to have had 
dealings with the Evil One. They learned his secrets in the Scholomance, 
amongst the mountains over Lake Hermanstadt, where the devil claims the 
tenth scholar as his due. In the records are such words as 
‘stregoica’—witch, ‘ordog,’ and ‘pokol’—Satan and hell; and in one 
manuscript this very Dracula is spoken of as ‘wampyr,’ which we all 
understand too well. There have been from the loins of this very one 
great men and good women, and their graves make sacred the earth where 
alone this foulness can dwell. For it is not the least of its terrors 
that this evil thing is rooted deep in all good; in soil barren of holy 
memories it cannot rest.”'


[1] "I desired that I might pass my life on that barren rock, wearily, 
it is true, but uninterrupted by any sudden shock of misery. If I 
returned, it was to be sacrificed or to see those whom I most loved die 
under the grasp of a daemon whom I had myself created."


--
 Dalibor Topic | Principal Product Manager
Phone: +494089091214  | Mobile: +491737185961


ORACLE Deutschland B.V. & Co. KG | Kühnehöfe 5 | 22761 Hamburg

ORACLE Deutschland B.V. & Co. KG
Hauptverwaltung: Riesstr. 25, D-80992 München
Registergericht: Amtsgericht München, HRA 95603

Komplementärin: ORACLE Deutschland Verwaltung B.V.
Hertogswetering 163/167, 3543 AS Utrecht, Niederlande
Handelsregister der Handelskammer Midden-Niederlande, Nr. 30143697
Geschäftsführer: Alexander van der Ven, Jan Schultheiss, Val Maher

 Oracle is committed to developing
practices and products that help protect the environment

Re: Optional.isEmpty()

2017-04-24 Thread Andrew Dinn 
On 22/04/17 14:31, Jonathan Bluett-Duncan wrote:
> Your reasoning has personally convinced me that a method like `isEmpty()`
> would pull its weight. However, at the risk of bikeshedding, I think it
> should be named differently, as `isEmpty()` immediately makes me think that
> `findModule()` returns a List, which I'd easily find confusing.
> 
> How about `isNotPresent()` instead?

Ah, bike-shedding!

Personally, I much prefer isAbsent() to isNotPresent(), presence and
absence being a historically well-sanctioned English language pairing.

[n.b. I'll grant that my preference for C18th literature over Comp Sci
argot might have swayed my judgement in this matter so I'll leave it to
others to sanction or reject that suggestion via a conformantly aligned,
textually rendered act of pollification (i.e. go on, post yer +/-1
follow-ups)]

regards,


Andrew Dinn
---
Senior Principal Software Engineer
Red Hat UK Ltd
Registered in England and Wales under Company Registration No. 03798903
Directors: Michael Cunningham, Michael ("Mike") O'Neill, Eric Shander

Re: Optional.isEmpty()

2017-04-22 Thread Jonathan Bluett-Duncan 
Hi Peter,

Your reasoning has personally convinced me that a method like `isEmpty()`
would pull its weight. However, at the risk of bikeshedding, I think it
should be named differently, as `isEmpty()` immediately makes me think that
`findModule()` returns a List, which I'd easily find confusing.

How about `isNotPresent()` instead?

Best regards,
Jonathan

On 22 April 2017 at 10:40, Peter Levart  wrote:

> Hi,
>
> Seeing the following line in some JDK test that was up for review:
>
> return cf.findModule(target).orElse(null) == null;
>
> I immediately jumped to suggest it would look better if written as:
>
> return !cf.findModule(target).isPresent();
>
>
> But then I leaned back and asked myself: "Would it really?"
>
> The boolean negation in front of a chain of method calls tends to visually
> disappear from the view when we finally reach the offending method and
> might get missed when quickly browsing the code. In this respect,
> ".orElse(null) == null" is actually better. But the best would of course be
> something like that:
>
> return cf.findModule(target).isEmpty();
>
> What do you think? Would this pull its weight?
>
> Regards, Peter
>
>

Optional.isEmpty()

2017-04-22 Thread Peter Levart 

Hi,

Seeing the following line in some JDK test that was up for review:

return cf.findModule(target).orElse(null) == null;

I immediately jumped to suggest it would look better if written as:

return !cf.findModule(target).isPresent();


But then I leaned back and asked myself: "Would it really?"

The boolean negation in front of a chain of method calls tends to 
visually disappear from the view when we finally reach the offending 
method and might get missed when quickly browsing the code. In this 
respect, ".orElse(null) == null" is actually better. But the best would 
of course be something like that:


return cf.findModule(target).isEmpty();

What do you think? Would this pull its weight?

Regards, Peter