I'm happy with this now, thanks Stephen
On 9 January 2016 at 12:44, nadeesh tv <nadeesh...@oracle.com> wrote: > Hi Stephen, > > It's already covered in > > public void test_plusDays_invalidTooLarge() and public void > test_minusDays_maximum(). > > Thanks and Regards, > Nadeesh > > > On 1/9/2016 4:58 PM, Stephen Colebourne wrote: >> >> Thanks for the update. You should have a test case for the exception >> thrown by the checkValidValue() >> Stephen >> >> On 9 January 2016 at 09:33, nadeesh tv <nadeesh...@oracle.com> wrote: >>> >>> Hi Stephen/Roger, >>> >>> Please see the updated the webrev >>> http://cr.openjdk.java.net/~ntv/8068803/webrev.03/ >>> >>> Explicit "YEAR.checkValidValue(year + 1);' added in 3rd case to handle >>> the >>> >>> invalidTooLarge year case (LocalDate.of(Year.MAX_VALUE, 12, >>> 31).plusDays(1)) >>> >>> Thanks and Regards, >>> Nadeesh >>> >>> On 1/9/2016 3:39 AM, Roger Riggs wrote: >>> >>> +1 >>> >>> (With Stephen's update below). >>> >>> Roger >>> >>> >>> On 1/8/2016 6:56 AM, Stephen Colebourne wrote: >>> >>> As I mentioned in my email: >>> >>> Rather than doing: >>> return withDayOfMonth((int) dom); >>> or >>> return LocalDate.of(year, month, (int) dom); >>> you can do >>> return new LocalDate(year, month, (int) dom); >>> >>> (there are two occurrences) >>> >>> Stephen >>> >>> >>> >>> On 8 January 2016 at 10:56, nadeesh tv <nadeesh...@oracle.com> wrote: >>> >>> Hi all, >>> >>> Thanks Stephen for the comments >>> Please see the updated webrev >>> http://cr.openjdk.java.net/~ntv/8068803/webrev.02/ >>> Regards, >>> Nadeesh >>> >>> On 1/7/2016 6:15 PM, Stephen Colebourne wrote: >>> >>> I updated the benchmark with this change and another one: >>> >>> >>> https://github.com/ThreeTen/threeten-bench/blob/master/src/main/java/org/threeten/LocalDateBenchmark.java >>> >>> Marginally fastest is this pattern as it avoids one if statement: >>> >>> if (dom > 0) { >>> if (dom <= 28) { // same month >>> return ... >>> } >>> if (dom <= 59) { // 59th Jan is 28th Feb >>> return ... >>> } >>> } >>> >>> Rather than doing: >>> return LocalDate.of(year, month, (int) dom); >>> we can also do >>> return new LocalDate(year, month, (int) dom); >>> >>> This is safe, because we know that the year, month and day are valid. >>> (I can't easily test the performance of this change, but it should be >>> noticeable as it avoids a lot of unnecessary checks). >>> >>> I'd like a few more test cases. Addition around 27/28/29/30 from the >>> first of Jan/Feb, and of 13/14/15/16 from the 15th of Jan and 15th of >>> Feb. >>> >>> Stephen >>> >>> >>> >>> On 7 January 2016 at 09:20, nadeesh tv <nadeesh...@oracle.com> wrote: >>> >>> Hi , >>> Please see the updated webrev >>> http://cr.openjdk.java.net/~ntv/8068803/webrev.01/ >>> Thanks and regards, >>> Nadeesh >>> >>> On 1/6/2016 12:11 AM, Roger Riggs wrote: >>> >>> Hi Nadeesh, >>> >>> LocalDate.java: +1374: >>> For the most common case of dom > 0 && <= 28, I would have >>> explicitly >>> and >>> immediately returned the new LocalDate. >>> >>> if (dom > 0 && dom <= 28) { >>> return LocalDate.of(year, month, (int) dom); >>> } >>> ... >>> >>> >>> TCKLocalDate.java: >>> - Since the test_plusDays_normal is being replaced, its test case >>> should be >>> included in the DataProvider >>> >>> {LocalDate.of(2007, 7, 15), 1, LocalDate.of(2007, 7, 16)} >>> >>> Thanks, Roger >>> >>> On 1/5/2016 1:05 PM, nadeesh tv wrote: >>> >>> Hi all, >>> >>> Please review a fix for >>> https://bugs.openjdk.java.net/browse/JDK-8068803 >>> web rev : http://cr.openjdk.java.net/~ntv/8068803/webrev.00/ >>> >>> Special thanks for Stephen for providing the source code patch >>> >>> >>> >>> -- >>> Thanks and Regards, >>> Nadeesh TV >>> >>> -- >>> Thanks and Regards, >>> Nadeesh TV >>> >>> >>> >>> -- >>> Thanks and Regards, >>> Nadeesh TV >>> > > -- > Thanks and Regards, > Nadeesh TV >
