RFR: JDK-8142936:Additional java.time.Duration methods
nadeesh tv
nadeesh.tv at oracle.com
Thu Dec 3 17:52:37 UTC 2015
Hi all,
Please see the updated webrev -
http://cr.openjdk.java.net/~ntv/8142936/webrev.03/
- changes - Modified the dataprovider as suggested by Roger
Thanks and regards,
Nadeesh
On 12/1/2015 2:39 AM, Stephen Colebourne wrote:
> This is all about fixing a messy API that was created in 8. The
> methods propose are all about consistency. The toSeconds() method
> completes the set. For each unit there is a toXxx() and a toXxxPart().
> Missing one out makes everything worse.
>
> Stephen
>
>
> On 30 November 2015 at 19:02, Roger Riggs <Roger.Riggs at oracle.com> wrote:
>> 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
>>>>
--
Thanks and Regards,
Nadeesh TV
More information about the core-libs-dev
mailing list