Agree this is better and cleaner!
Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com Sent from my iPad On Jun 21, 2014, at 4:27 AM, Daniel Fuchs <daniel.fu...@oracle.com> wrote: > Thanks Joe! > > This is much cleaner indeed :-) > > -- daniel > > On 6/21/14 4:36 AM, huizhe wang wrote: >> Thanks Daniel, Lance. >> >> On 6/20/2014 3:02 AM, Daniel Fuchs wrote: >>> Hi Joe, >>> >>> Thanks for the detailed explanation. >>> It really helps reviewing the fix! >> >> Glad to know it helps. I thought this part of spec could be unfamiliar to >> people. >> >>> >>> This looks reasonable to me. One minor nit is that you >>> could turn: >>> >>> 769 BigInteger maxintAsBigInteger = BigInteger.valueOf((long) >>> Integer.MAX_VALUE); >> >> Good catch! I was going to do so but then forgot. >> >> I also refactored the check method so that the checks can be done in one >> loop: 24 lines of code instead of the original 170. I feel good :-) >> >> The other changes are purely clean-up and in one case, JavaDoc. >> >> http://cr.openjdk.java.net/~joehw/jdk9/5077522/webrev/ >> >> Best regards, >> Joe >> >> >>> >>> into a static final constant in the class. >>> >>> best regards, >>> >>> -- daniel >>> >>> On 6/17/14 9:19 PM, huizhe wang wrote: >>>> Hi, >>>> >>>> This is a long time compatibility issue: Duration.compare returns equal >>>> for INDETERMINATE relations defined in XML Schema standard >>>> (http://www.w3.org/TR/xmlschema-2/#duration-order) as listed in the >>>> following table: >>>> >>>> Relation >>>> P*1Y* > P*364D* <> P*365D* <> P*366D* < P*367D* >>>> P*1M* > P*27D* <> P*28D* <> P*29D* <> P*30D* <> >>>> P*31D* < P*32D* >>>> P*5M* > P*149D* <> P*150D* <> P*151D* <> P*152D* <> >>>> P*153D* < P*154D* >>>> >>>> >>>> >>>> The order-relation of two Duratoin values x and y is x < y iff s+x < s+y >>>> for each qualified datetime s listed below: >>>> >>>> * 1696-09-01T00:00:00Z >>>> * 1697-02-01T00:00:00Z >>>> * 1903-03-01T00:00:00Z >>>> * 1903-07-01T00:00:00Z >>>> >>>> >>>> The original implementation used Unix epoch, that is, 00:00:00 UTC on 1 >>>> January 1970, as s in the above calculation which violated the above >>>> specification. A patch during JDK 6 development added correct >>>> implementation of the spec, but it was unfortunately added after the >>>> original calculation using Epoch time. >>>> >>>> *The fix to the issue therefore is simply removing the calculation using >>>> Epoch time.* I also consolidated the tedious max field value checks into >>>> a method called checkMaxValue. >>>> >>>> *Patch:* >>>> http://cr.openjdk.java.net/~joehw/jdk9/5077522/webrev/ >>>> >>>> Test: >>>> testCompareWithInderterminateRelation: this is a copy of the JCK test >>>> that tests INDETERMINATE relations. >>>> testVerifyOtherRelations: this is added to verify edge cases, e.g. +- 1 >>>> second to the original test cases. For example, to the original test: >>>> PT525600M is P365D <> P1Y, I added "PT525599M59S", "<", "P1Y", and >>>> PT527040M -> P366D <> P1Y, "PT527040M1S", ">", "P1Y" >>>> >>>> Below is the test result: >>>> Comparing P1Y and P365D: INDETERMINATE >>>> Comparing P1Y and P366D: INDETERMINATE >>>> Comparing P1M and P28D: INDETERMINATE >>>> Comparing P1M and P29D: INDETERMINATE >>>> Comparing P1M and P30D: INDETERMINATE >>>> Comparing P1M and P31D: INDETERMINATE >>>> Comparing P5M and P150D: INDETERMINATE >>>> Comparing P5M and P151D: INDETERMINATE >>>> Comparing P5M and P152D: INDETERMINATE >>>> Comparing P5M and P153D: INDETERMINATE >>>> Comparing PT2419200S and P1M: INDETERMINATE >>>> Comparing PT2678400S and P1M: INDETERMINATE >>>> Comparing PT31536000S and P1Y: INDETERMINATE >>>> Comparing PT31622400S and P1Y: INDETERMINATE >>>> Comparing PT525600M and P1Y: INDETERMINATE >>>> Comparing PT527040M and P1Y: INDETERMINATE >>>> Comparing PT8760H and P1Y: INDETERMINATE >>>> Comparing PT8784H and P1Y: INDETERMINATE >>>> Comparing P365D and P1Y: INDETERMINATE >>>> Comparing P1Y and P364D: expected: GREATER actual: GREATER >>>> Comparing P1Y and P367D: expected: LESSER actual: LESSER >>>> Comparing P1Y2D and P366D: expected: GREATER actual: GREATER >>>> Comparing P1M and P27D: expected: GREATER actual: GREATER >>>> Comparing P1M and P32D: expected: LESSER actual: LESSER >>>> Comparing P1M and P31DT1H: expected: LESSER actual: LESSER >>>> Comparing P5M and P149D: expected: GREATER actual: GREATER >>>> Comparing P5M and P154D: expected: LESSER actual: LESSER >>>> Comparing P5M and P153DT1H: expected: LESSER actual: LESSER >>>> Comparing PT2419199S and P1M: expected: LESSER actual: LESSER >>>> Comparing PT2678401S and P1M: expected: GREATER actual: GREATER >>>> Comparing PT31535999S and P1Y: expected: LESSER actual: LESSER >>>> Comparing PT31622401S and P1Y: expected: GREATER actual: GREATER >>>> Comparing PT525599M59S and P1Y: expected: LESSER actual: LESSER >>>> Comparing PT527040M1S and P1Y: expected: GREATER actual: GREATER >>>> Comparing PT8759H59M59S and P1Y: expected: LESSER actual: LESSER >>>> Comparing PT8784H1S and P1Y: expected: GREATER actual: GREATER >>>> >>>> Number of tests passed: 36 >>>> Number of tests failed: 0 >>>> >>>> Thanks, >>>> Joe >
