harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jimmy,Jing Lv" <firep...@gmail.com>
Subject Re: [classlib][luni] Set and get value for java.util.GregorianCalendar.ZONE_OFFSET
Date Wed, 16 Apr 2008 10:55:51 GMT
Hi Jim,

    The fix looks fine to me, if no objection, I'll apply it.

2008/4/16, Jim Yu <junjie0122@gmail.com>:
> Hi, all,
>  I've submitted my patch to this JIRA. At
>
> https://issues.apache.org/jira/browse/HARMONY-2422
>
> Anybody to verify it? Thanks Tony :-)
>
>  2008/4/16, Tony Wu <wuyuehao@gmail.com>:
>
> >
>  > Hi, Jim
>  > Thanks very much for your investiagtion and patch. please feel free to
>  > attach your fix to this JIRA and grant the license to apache... so
>  > that committers can integrate your patch. thanks
>  >
>  > On 4/16/08, Jim Yu <junjie0122@gmail.com> wrote:
>  > > Hi, all,
>  > >
>  > > I've just looked at this issue from a JIRA at
>  > > https://issues.apache.org/jira/browse/HARMONY-2422.
>  > > If we try to set an arbitrary value to ZONE_OFFSET field, the returned
>  > value
>  > > by get method will not be changed.
>  > > It is a raw offset from GMT in milliseconds depends on your time zone.
>  > But
>  > > RI doesn't have such behavior. It will
>  > > return the value you just set.
>  > >
>  > > Test to reproduce (from the reporter):
>  > >
>  > > import java.util.GregorianCalendar;
>  > > import junit.framework.TestCase;
>  > >
>  > > public class Test extends TestCase {
>  > >
>  > >    public void testcase() {
>  > >        GregorianCalendar gc = new GregorianCalendar(1, 1, 1);
>  > >        System.out.println("gc = " + gc);
>  > >        gc.set(GregorianCalendar.ZONE_OFFSET, -1);
>  > >        System.out.println("gc.get(" + GregorianCalendar.ZONE_OFFSET + ")
>  > =
>  > > "
>  > >                + gc.get(GregorianCalendar.ZONE_OFFSET));
>  > >        assertTrue("gc.get(" + GregorianCalendar.ZONE_OFFSET + ") == -1",
>  > > (gc
>  > >                .get(GregorianCalendar.ZONE_OFFSET) == -1));
>  > >    }
>  > >
>  > > }
>  > >
>  > > Output on Harmony:
>  > > There was 1 failure:
>  > > 1) testcase(Test)junit.framework.AssertionFailedError: gc.get(15) == -1
>  > >        at Test.testcase(Test.java:12)
>  > >        at java.lang.reflect.VMReflection.invokeMethod(Native Method)
>  > >
>  > > FAILURES!!!
>  > > Tests run: 1, Failures: 1, Errors: 0
>  > >
>  > > res = 1
>  > >
>  > > Output on RI:
>  > > OK (1 test)
>  > >
>  > > res = 0
>  > >
>  > > On Harmony, we will always get a constant value(the specific value
>  > depends
>  > > on your time zone) for ZONE_OFFSET field
>  > > after setting an arbitrary value to it. The reson is that, in
>  > computeFields
>  > > method, ZONE_OFFSET field will always be set.
>  > > According to the spec, complete method will only fills in any unset
>  > fields
>  > > in the calender fields. So I think it is not correct
>  > > to set ZONE_OFFSET again in computeFields method if we have set it
>  > already.
>  > > I suggest to add an if statement to judge whether
>  > > the ZONE_OFFSET field has been set before setting its value. We can use
>  > > isSet[ZONE_OFFSET] to determine whether the field has
>  > > been set. Since when we set a field value, the associated item in isSet
>  > > boolean array will be set true. I've pasted the patch at the
>  > > bottom. Any suggestion?
>  > >
>  > > Source code about this issue:
>  > > Calender.java:
>  > >
>  > >  public void set(int field, int value) {
>  > >        fields[field] = value;
>  > >        isSet[field] = true;
>  > >        areFieldsSet = isTimeSet = false;
>  > >        ...
>  > >  }
>  > >  public int get(int field) {
>  > >        complete();
>  > >        return fields[field];
>  > >    }
>  > >
>  > >  protected void complete() {
>  > >        ...
>  > >        if (!areFieldsSet) {
>  > >            computeFields();
>  > >            areFieldsSet = true;
>  > >        }
>  > >    }
>  > > GregorianCalendar.java:
>  > >    protected void computeFields() {
>  > >        int zoneOffset = getTimeZone().getRawOffset();
>  > >
>  > >        fields[ZONE_OFFSET] = zoneOffset;
>  > >        ...
>  > >    }
>  > >
>  > >
>  > > Here is my patch:
>  > > Index: src/main/java/java/util/GregorianCalendar.java
>  > > ===================================================================
>  > > --- src/main/java/java/util/GregorianCalendar.java (revision 648485)
>  > > +++ src/main/java/java/util/GregorianCalendar.java (working copy)
>  > > @@ -446,7 +446,9 @@
>  > >     protected void computeFields() {
>  > >         int zoneOffset = getTimeZone().getRawOffset();
>  > >
>  > > -        fields[ZONE_OFFSET] = zoneOffset;
>  > > +        if(!isSet[ZONE_OFFSET]) {
>  > > +            fields[ZONE_OFFSET] = zoneOffset;
>  > > +        }
>  > >
>  > >         int millis = (int) (time % 86400000);
>  > >         int savedMillis = millis;
>  > >
>  > >
>  > > --
>  > > Best Regards,
>  > > Jim Yu (虞俊杰)
>  > >
>  > > China Software Development Lab, IBM
>  > >
>  >
>  >
>  > --
>  > Tony Wu
>  > China Software Development Lab, IBM
>  >
>
>
>
>
> --
>
> Best Regards,
>  Jim Yu (虞俊杰)
>
>  China Software Development Lab, IBM
>


-- 

Best Regards!

Jimmy, Jing Lv
China Software Development Lab, IBM
Mime
View raw message