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

[Stephen Colebourne]

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.

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?)

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
>>>
>>>
>>>
>