I think this might be clearer: * Checks if the specified year is a leap year. * <p> * A leap-year is a year of a longer length than normal. * The exact meaning is determined by the chronology according to the following constraints. * <ul> * <li>a leap-year must imply a year-length longer than a non leap-year. * <li>a chronology that does not support the concept of a year must return false. * <li>the correct result must be returned for all years within the valid range of years for the chronology * </ul> * Outside the range of valid years an implementation is free to return either a best guess or false. * An implementation must not throw an exception, even if the year is outside the range of valid years..
Stephen On 4 February 2015 at 19:08, Roger Riggs <roger.ri...@oracle.com> wrote: > Hi, > > Clarifying that Chronology.isLeapYear is specified only within the range of > the chronology > makes it possible to maintain the invariants with the calendar computations > and methods. > Best effort isn't testable except in an implementation specific way and > can't be relied on. > > The other two constraints are testable and use 'must' in their definitions. > The new phrasing should clearly be either an exception or reinforce the > value is > specified only within the range of the chronology. > > How about: > > <li>except if the year is out of range the chronology should return a best > effort guess, or false if there is no suitable guess > > Roger > > > > On 2/4/2015 11:18 AM, Stephen Colebourne wrote: >> >> 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 >>>>>>> >>>>>>> >>>>>>> >
