The spec is pretty clear: "the proleptic-year to check, not validated for range"
Adding an exception to the spec would cause requests to add exceptions to all other chronologies. Doing so, would be very negative to performance (it would require having a non-public copy of the logic elsewhere to avoid the range checking cost. Adding an exception to Hijrah only means that the implementation is doing something not permitted by the spec. The only sane choice here is to return false from Hijrah when out of range. As intended and as specced. Note that I don't believe that returning false will cause hardship in any actual user code, because other methods will constrain the year to be valid. If necessary, the following spec clarification could be added to the bullet points on Chronology: <li>if the year is out of range the chronology should return a best effort guess, or false if there is no suitable guess Stephen On 4 February 2015 at 16:05, Roger Riggs <roger.ri...@oracle.com> wrote: > 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.ri...@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.ander...@oracle.com> >>>> wrote: >>>>> >>>>> +1 >>>>> On Feb 3, 2015, at 3:45 PM, Roger Riggs <roger.ri...@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.ander...@oracle.com >>>>> >>>>> >>>>> >
