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.ri...@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...@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...@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

Reply via email to