RFR: JDK-8142936:Additional java.time.Duration methods

[Roger Riggs]

Hi Stephen, Nadeesh,

The toXXXPart methods look fine.

I'm not entirely convinced that the toSeconds() method is worth it and 
may be
deemed as confusing with the new toSecondsPart method.
How many people have problems with getSeconds()?

The toXXXPart tests could have used a single DataProvider with
the values supplied for each unit to reduce the duplication.

Thanks,  Roger




On 11/30/2015 09:29 AM, Stephen Colebourne wrote:
> I think thats ready to be merged, thanks
> Stephen
>
> On 30 November 2015 at 11:26, nadeesh tv <nadeesh.tv at oracle.com> wrote:
>> Hi all,
>> Please see the updated webrev
>> http://cr.openjdk.java.net/~ntv/8142936/webrev.02/
>> Regards,
>> Nadeesh TV
>>
>> On 11/27/2015 11:20 PM, Stephen Colebourne wrote:
>>> "Overall, looks pretty good.
>>>
>>> There a a number of double spaces that should be single spaces in the
>>> Javadoc.
>>>
>>> Change
>>> "This is based on the standard definition of a day has 24 hours."
>>> to
>>> "This is based on the standard definition of a day as 24 hours."
>>> ("has" to "as")
>>> There are three places to fix this.
>>>
>>> The toMillisPart() docs could do with some rework.
>>> change
>>> "number of milliseconds part in the nanosecond part of the duration"
>>> to
>>> "number of milliseconds part of the duration"
>>>
>>> try this:
>>> "This returns the milliseconds part by dividing the number of
>>> nanoseconds by 1,000,000."
>>>
>>> On the tests.
>>>
>>> There is no test for toSeconds().
>>>
>>> For the other tests, I have been bitten before by not testing edge cases.
>>> A test for zero, and for a negative round number would be good.
>>> eg. for toHoursPart() the round number would be a duration of exactly
>>> minus 2 hours.
>>>
>>> Duration.ofHours(2).toDaysPart() = 0
>>> Duration.ofHours(2).toHoursPart() = 2
>>> Duration.ofHours(2).toMinutesPart() = 0
>>> Duration.ofHours(2).toSecondsPart() = 0
>>> Duration.ofHours(2).toMillisPart() = 0
>>> Duration.ofHours(2).toNanosPart() = 0
>>>
>>> thus really six tests are needed each time. The best way to achieve
>>> this is using @DataProvider in TestNG, where you can setup a data grid
>>> of inputs and 6 expected outputs.
>>>
>>> thanks
>>> Stephen
>>>
>>>
>>>
>>>
>>> On 26 November 2015 at 06:41, nadeesh tv <nadeesh.tv at oracle.com> wrote:
>>>> Hi all,
>>>>
>>>> Please review a fix for
>>>>
>>>> Bug Id - https://bugs.openjdk.java.net/browse/JDK-8142936
>>>>
>>>>    -Enhance Duration by adding toNanosPart() ,
>>>> toMillisPart(),toSecondsPart(),toMinutesPart(),toHoursPart(),toDaysPart()
>>>> methods .
>>>>    - Had to rename  private    BigDecimal toSeconds() ->    private
>>>> BigDecimal
>>>> toBigDecimalSeconds()
>>>>
>>>>
>>>> webrev - http://cr.openjdk.java.net/~ntv/8142936/webrev.01/
>>>>
>>>> --
>>>> Thanks and Regards,
>>>> Nadeesh TV
>>>> <div id="DDB4FAA8-2DD7-40BB-A1B8-4E2AA1F9FDF2"><table style="border-top:
>>>> 1px solid #aaabb6; margin-top: 10px;">
>>>          <tr>
>>>                  <td style="width: 105px; padding-top: 15px;">
>>>                          <a
>>> href="https://www.avast.com/?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail"
>>> target="_blank"><img
>>> src="https://ipmcdn.avast.com/images/logo-avast-v1.png" style="width:
>>> 90px; height:33px;"/></a>
>>>                  </td>
>>>                  <td style="width: 470px; padding-top: 20px; color:
>>> #41424e;
>>> font-size: 13px; font-family: Arial, Helvetica, sans-serif;
>>> line-height: 18px;">This email has been sent from a virus-free
>>> computer protected by Avast. <br /><a
>>>
>>> href="https://www.avast.com/?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail"
>>> target="_blank" style="color: #4453ea;">www.avast.com</a>
>>>                  </td>
>>>          </tr>
>>> </table><a href="#DDB4FAA8-2DD7-40BB-A1B8-4E2AA1F9FDF2" width="1"
>>> height="1"></a></div>
>>
>> --
>> Thanks and Regards,
>> Nadeesh TV
>>