Duration Java 9 methods #410
Conversation
| * the number of seconds parts in the duration, may be negative | ||
| * @since 9 | ||
| */ | ||
| def toSecondsPart: Int = (toSeconds / SECONDS_PER_MINUTE).toInt |
There was a problem hiding this comment.
Hmm, shouldn't this be using % to get the remainder instead of / ? Why doesn't the test suite catch this?
There was a problem hiding this comment.
Oh right, it say so on the docs 😄 the test suite is not good enough
There was a problem hiding this comment.
Great catch I added a fix
| * the number of hours part in the duration, may be negative | ||
| * @since 9 | ||
| */ | ||
| def toHoursPart: Int = (toHours / HOURS_PER_DAY).toInt |
There was a problem hiding this comment.
They should all be remainders except toMillis
|
@armanbilge I amended the implementation and added new tests to surface what was working wrong earlier in case you want to take a look |
| * @return | ||
| * the nanoseconds within the second part of the length of the duration, from 0 to 999,999,999 | ||
| * @since 9 | ||
| */ | ||
| def toNanosPart: Int = (nanos / Duration.NANOS_PER_SECOND).toInt |
There was a problem hiding this comment.
Sorry, I still don't understand the calculation here. According to the constructor, nanos should always be between 0 and 999,999,999. So why do we need to do any calculation here?
scala-java-time/core/shared/src/main/scala/org/threeten/bp/Duration.scala
Lines 495 to 501 in bee7f13
There was a problem hiding this comment.
Right, let's delete toNanosPart and release, If somebody needs it PRs are welcome
There was a problem hiding this comment.
Hm, I don't think it needed to be deleted. I think the correct implementation is to just return nanos here :)
cquiroz
force-pushed
the
duration-9-methods
branch
from
a1ad3e9 to
0f5f39a
Compare
cquiroz
force-pushed
the
duration-9-methods
branch
from
0f5f39a to
d2da535
Compare
Partially fixes #254