RFR 9: 8067800: Unexpected DateTimeException in the java.time.chrono.HijrahChronology.isLeapYear

[Roger Riggs]

Hi Stephen,

On 2/4/15 10:43 AM, Stephen Colebourne wrote:
> The java.time code generally takes a lenient approach to methods that
> return a boolean. For example, they tend to accept null inputs without
> throwing an exception.
Seems like an odd design provision leading to some behavior that
would be inconsistent with non-boolean methods.
>
> Right now, this patch makes a subclass implementation incompatible
> with the superclass spec. That cannot happen. Thus there are only two
> options:
>
> - change the superclass spec
> - return false from the subclass
>
> I favour the latter as being in line with the rest of the package and
> less disruptive to existing users (would a CCC even pass?)
For a 'young' API with limited uptake, issues can be fixed early and in 
a major
release there is more flexibility to clarify the specification.

DTE is a runtime exception and can occur in a majority of time computations.
In the case of HijrahChronolog.isLeapYear, the implementation currently
throws a different RuntimeException and this would be a correction
to the conventional exception.

Roger

>
> Stephen
>
>
> On 4 February 2015 at 15:10, Roger Riggs <Roger.Riggs at oracle.com> wrote:
>> Hi Stephen,
>>
>> I also indicated in the Jira comments that it is misleading and incorrect to
>> return
>> false when it is not known that a year is or is not a leap year. All of the
>> other
>> HijrahChronology computations throw DTE for invalid dates and the same may
>> be
>> true for other Chronologies.
>>
>> The assertion in Chronology.isLeapYear about not checking the range is too
>> broad
>> and should be qualified by the Chronology.
>>
>> Perhaps the proposed change should include a caveat in that method.
>>
>> Roger
>>
>>
>>
>>
>> On 2/3/15 8:05 PM, Stephen Colebourne wrote:
>>> -1
>>>
>>> As I indicated on JIRA, I don't believe that this change meets the
>>> spec or intent of the definition on Chronology. That is specified to
>>> not throw any exceptions and to handle all years, valid or not.
>>>
>>> I don't foresee any significant issue where a year is not validated by
>>> this method. Years out of range should simply return false, again
>>> something that is within the spirit of the spec "a chronology that
>>> does not support the concept of a year must return false."
>>>
>>> Stephen
>>>
>>>
>>>
>>> On 3 February 2015 at 20:56, Lance Andersen <lance.andersen at oracle.com>
>>> wrote:
>>>> +1
>>>> On Feb 3, 2015, at 3:45 PM, Roger Riggs <Roger.Riggs at oracle.com> wrote:
>>>>
>>>>> Please review this specification clarification of the range of Hijrah
>>>>> calendar variants.
>>>>> The issue was exposed by a bug in the HijrahChronology.isLeapYear
>>>>> method.
>>>>>
>>>>> Webrev:
>>>>>     http://cr.openjdk.java.net/~rriggs/webrev-leap-year-8067800/
>>>>>
>>>>> Issue:
>>>>>     https://bugs.openjdk.java.net/browse/JDK-8067800
>>>>>
>>>>> A CCC may be needed.
>>>>>
>>>>> Thanks, Roger
>>>>>
>>>>
>>>> Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
>>>> Oracle Java Engineering
>>>> 1 Network Drive
>>>> Burlington, MA 01803
>>>> Lance.Andersen at oracle.com
>>>>
>>>>
>>>>