RFR: 8184693: (opt) add Optional.isEmpty

[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.theeyarath at oracle.com>
Cc: core-libs-dev <core-libs-dev at openjdk.java.net>; Paul Sandoz <paul.sandoz at 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.theeyarath at oracle.com>
> Cc: core-libs-dev <core-libs-dev at openjdk.java.net>; Paul Sandoz 
> <paul.sandoz at 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.theeyarath at oracle.com>
>> Cc: Remi Forax <forax at univ-mlv.fr>; core-libs-dev 
>> <core-libs-dev at openjdk.java.net>
>> Subject: Re: RFR: 8184693: (opt) add Optional.isEmpty
>>
>>
>>
>>> On 15 Apr 2018, at 11:25, Vivek Theeyarath <vivek.theeyarath at 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 <forax at univ-mlv.fr>
>>> Cc: core-libs-dev <core-libs-dev at 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:forax at univ-mlv.fr]
>>> Sent: Saturday, April 14, 2018 2:58 PM
>>> To: Vivek Theeyarath <vivek.theeyarath at oracle.com>
>>> Cc: core-libs-dev <core-libs-dev at 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.theeyarath at oracle.com>
>>>> À: "core-libs-dev" <core-libs-dev at 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
>>