Hi Roger, I think I know what this is trying to say, but if possible, could you provide the revised javadoc so it is easier to see the full context?
Best Lance On Feb 4, 2015, at 2:08 PM, 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 >>>>>>> >>>>>>> >>>>>>> > Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
