"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>
![https://ipmcdn.avast.com/images/logo-avast-v1.png"](https://ipmcdn.avast.com/images/logo-avast-v1.png"){kind=link}