RFR: JDK-8142936:Additional java.time.Duration methods
nadeesh tv
nadeesh.tv at oracle.com
Fri Dec 4 11:04:31 UTC 2015
Hi ,
Thanks for the comments.
Please see the updated webrev
http://cr.openjdk.java.net/~ntv/8142936/webrev.04/
Thanks and Regards,
Nadeesh
On 12/4/2015 3:12 AM, Roger Riggs wrote:
> Hi Nadeesh,
>
> Thanks for the update, sorry I missed some editorial comments last time.
>
> toSeconds method missing a final period '.'
> "* This returns the total number of seconds in the duration"
> toMinutesPart method: Extra text:
> + * @return the number of minutes parts in the duration, may be negative
> + * @since 9
> *+ * may be negative*
> + */
> + public int toMinutesPart(){
>
>
> Missing space in multiple places: "(){" should have a space before "{"
>
>
> toMillisPart and toNanosPart methods: unneeded description:
> "+ * The total duration is defined by calling {@link #getNano()} and
> {@link #getSeconds()}."
>
> toMillisPart method: remove the final "."; it should be omitted on
> @return, @param
> "+ * @return the number of milliseconds part of the duration."
>
> Thanks for coalescing the data providers.
>
> Roger
>
>
>
> On 12/03/2015 12:52 PM, nadeesh tv wrote:
>> 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