RFR:JDK-8030864:Add an efficient getDateTimeMillis method to java.time
Roger Riggs
Roger.Riggs at Oracle.com
Fri Mar 11 15:49:35 UTC 2016
Hi Nadeesh,
Thanks for filling in the missing DateTimeException cases.
- src/java.base/share/classes/java/time/chrono/IsoChronology.java:
" * @throws DateTimeException if the value of any field is out of range,
+ * or if the day-of-month is invalid for the month-of-year"
refers to 'fields' but the values being checked are arguments.
Perhaps: * @throws DateTimeException if the value of any argument is
out of range,
+ * or if the day-of-month is invalid for the
month-of-year
- test/java/time/tck/java/time/chrono/TCKChronology.java:
The new test_bad_epochSecond has unused code:
+ ChronoLocalDate chronoLd = chrono.date(y, m, d);
If y, m, or d were out of range chrono.date would throw the expected
exception instead of epochSecond.
- test/java/time/tck/java/time/chrono/TCKIsoChronology.java
Since IsoChronology has completely different implementation,
test_epochSecond_bad() should
include out of range values for each or m,d,h,m,s.
Thanks, Roger
On 3/10/2016 4:53 AM, nadeesh tv wrote:
> Hi all,
>
> Please see the updated webrev
>
> http://cr.openjdk.java.net/~ntv/8030864/webrev.07/
>
> Changes:
> + @throws DateTimeException if any of the values are out of range
> in Chronology.epochSecond()
>
> and
> new test cases related to excepted exception in
> TCKChronology.test_bad_epochSecond()
>
> Thanks and Regards,
> Nadeesh TV
>
>
>
>
> On 3/8/2016 4:14 AM, Roger Riggs wrote:
>> Look fine.
>>
>> Roger
>>
>>
>> On 3/5/2016 7:05 AM, nadeesh tv wrote:
>>> Hi all,
>>>
>>> Please see the updated webrev
>>> http://cr.openjdk.java.net/~ntv/8030864/webrev.06/
>>>
>>>
>>> Regards,
>>> Nadeesh
>>> On 3/4/2016 4:34 PM, Stephen Colebourne wrote:
>>>> long DAYS_0000_TO_1970 should be extracted as a private static
>>>> final constant.
>>>>
>>>> Otherwise looks good.
>>>> Stephen
>>>>
>>>>
>>>> On 3 March 2016 at 18:54, nadeesh tv <nadeesh.tv at oracle.com> wrote:
>>>>> Hi,
>>>>>
>>>>> Roger - Thanks for the comments
>>>>>
>>>>> Made the necessary changes in the spec
>>>>>
>>>>> Please see the updated webrev
>>>>> http://cr.openjdk.java.net/~ntv/8030864/webrev.05/
>>>>> On 3/3/2016 12:21 AM, nadeesh tv wrote:
>>>>>> Hi ,
>>>>>>
>>>>>> Please see the updated webrev
>>>>>> http://cr.openjdk.java.net/~ntv/8030864/webrev.03/
>>>>>>
>>>>>> Thanks and Regards,
>>>>>> Nadeesh
>>>>>>
>>>>>> On 3/3/2016 12:01 AM, Roger Riggs wrote:
>>>>>>> Hi Nadeesh,
>>>>>>>
>>>>>>> Editorial comments:
>>>>>>>
>>>>>>> Chronology.java: 716+
>>>>>>> "Java epoch" -> "epoch"
>>>>>>> "minute, second and zoneOffset" -> "minute, second*,* and
>>>>>>> zoneOffset"
>>>>>>> (add a comma; two places)
>>>>>>>
>>>>>>> "caluculated using given era, prolepticYear," -> "calculated
>>>>>>> using the
>>>>>>> era, year-of-era,"
>>>>>>> "to represent" -> remove as unnecessary in all places
>>>>>>>
>>>>>>> IsoChronology:
>>>>>>> "to represent" -> remove as unnecessary in all places
>>>>>>>
>>>>>>> These should be fixed to cleanup the specification.
>>>>>>>
>>>>>>> The implementation and the tests look fine.
>>>>>>>
>>>>>>> Thanks, Roger
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On 3/2/2016 10:17 AM, nadeesh tv wrote:
>>>>>>>> Hi,
>>>>>>>> Stephen, Thanks for the comments.
>>>>>>>> Please see the updated webrev
>>>>>>>> http://cr.openjdk.java.net/~ntv/8030864/webrev.02/
>>>>>>>>
>>>>>>>> Regards,
>>>>>>>> Nadeesh TV
>>>>>>>>
>>>>>>>> On 3/2/2016 5:41 PM, Stephen Colebourne wrote:
>>>>>>>>> Remove "Subclass can override the default implementation for a
>>>>>>>>> more
>>>>>>>>> efficient implementation." as it adds no value.
>>>>>>>>>
>>>>>>>>> In the default implementation of
>>>>>>>>>
>>>>>>>>> epochSecond(Era era, int yearofEra, int month, int dayOfMonth,
>>>>>>>>> int hour, int minute, int second, ZoneOffset zoneOffset)
>>>>>>>>>
>>>>>>>>> use
>>>>>>>>>
>>>>>>>>> prolepticYear(era, yearOfEra)
>>>>>>>>>
>>>>>>>>> and call the other new epochSecond method. See dateYearDay(Era
>>>>>>>>> era,
>>>>>>>>> int yearOfEra, int dayOfYear) for the design to copy. If this
>>>>>>>>> is done,
>>>>>>>>> then there is no need to override the method in IsoChronology.
>>>>>>>>>
>>>>>>>>> In the test,
>>>>>>>>>
>>>>>>>>> LocalDate.MIN.with(chronoLd)
>>>>>>>>>
>>>>>>>>> could be
>>>>>>>>>
>>>>>>>>> LocalDate.from(chronoLd)
>>>>>>>>>
>>>>>>>>> Thanks
>>>>>>>>> Stephen
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 2 March 2016 at 10:30, nadeesh tv <nadeesh.tv at oracle.com>
>>>>>>>>> wrote:
>>>>>>>>>> Hi all,
>>>>>>>>>>
>>>>>>>>>> Please review an enhancement for a garbage free epochSecond
>>>>>>>>>> method.
>>>>>>>>>>
>>>>>>>>>> Bug ID: https://bugs.openjdk.java.net/browse/JDK-8030864
>>>>>>>>>>
>>>>>>>>>> webrev: http://cr.openjdk.java.net/~ntv/8030864/webrev.01
>>>>>>>>>>
>>>>>>>>>> --
>>>>>>>>>> Thanks and Regards,
>>>>>>>>>> Nadeesh TV
>>>>>>>>>>
>>>>> --
>>>>> Thanks and Regards,
>>>>> Nadeesh TV
>>>>>
>>>
>>
>
More information about the core-libs-dev
mailing list