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 >
