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

[Roger Riggs]

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
>>>>>
>