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