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

Reply via email to