javacalendar

java calendar returns wrong week


I have the following test that from my understanding should pass. Is there something I'm missing or is this a bug in Calendar?

Calendar inputC = new GregorianCalendar(TimeZone.getTimeZone("UTC"), Locale.US);  

// Sunday  
inputC.set(2014, Calendar.JUNE, 22, 0, 0, 0);  
inputC.set(Calendar.MILLISECOND, 0);  

// START code impl
// Given a date returns back the date with it's day being first day of week and resets time.  
Calendar dc = Calendar.getInstance(TimeZone.getTimeZone("UTC"), Locale.US);  
dc.set(inputC.get(Calendar.YEAR), inputC.get(Calendar.MONTH), inputC.get(Calendar.DAY_OF_MONTH), 0, 0 , 0);  
dc.set(Calendar.MILLISECOND, 0);  
// dc.getTimeInMillis();  
// dc.set(Calendar.WEEK_OF_YEAR, inputC.get(Calendar.WEEK_OF_YEAR));  
dc.set(Calendar.DAY_OF_WEEK, dc.getFirstDayOfWeek());  

Date output = dc.getTime();  
// END code impl

Calendar expectedSundayC = new GregorianCalendar(TimeZone.getTimeZone("UTC"), Locale.US);  
expectedSundayC.set(2014, Calendar.JUNE, 22, 0, 0, 0);  
expectedSundayC.set(Calendar.MILLISECOND, 0);  

assertEquals(output, expectedSundayC.getTime());  

Output:     2014-06-15T00:00:00Z
Expected: 2014-06-22T00:00:00Z

The above test fails unless I uncomment either one of the 2 following lines:

// dc.getTimeInMillis();  
// dc.set(Calendar.WEEK_OF_YEAR, inputC.get(Calendar.WEEK_OF_YEAR));

Why is that dc.getTimeInMillis() affects the output?
Uncommenting line 2 above seems redundant because I already set year,month,day,hour,minute,seconds, milliseconds fields which should be enough data for complete datetime.


Solution

  • You certainly gave me a headscratcher on this one, but I figured it out.

    Specifications

    From the JavaDoc:

    The calendar field values can be set by calling the set methods. Any field values set in a Calendar will not be interpreted until it needs to calculate its time value (milliseconds from the Epoch) or values of the calendar fields. Calling the get, getTimeInMillis, getTime, add and roll involves such calculation.

    What this means is that, whenever you change your values using set, you should tell it to update itself by using one of the get methods to propagate changes. However, your code didn't, except when you uncommented the code. So something in your code was obviously not playing well with your sets in the computeTime.

    Where Things Went Bad

    So... we can look at this out-of-date but nevertheless still monstrously hard-to-navigate copy of GregorianCalendar source and analyze exactly what's happening when you do the getTime() after setting the day of week with all the other chanes.

    Ugh, copying from that is a pain. I'll walk you through it.

    First, we basically know that getTimeInMillis() calls computeTime() at line 505.

    You declared an instance of Calendar. It has predefined values. Today's date to be exact. And also we are in the third week of September. Important later!

    Now we assign your day at line 511. This is where your day is set to 22.

    int day = fields[DAY_OF_MONTH];
    

    Line 523 is where things go bonkers and break your date! We evaluate this to be false and go to the else at line 553.

    if (! isSet[MONTH] && (! isSet[DAY_OF_WEEK] || isSet[WEEK_OF_YEAR]))
    //evaluates to true
    

    We enter the if statement at 555 because DAY_OF_WEEK is set. And we have a DAY_OF_WEEK_IN_MONTH, but we didn't set it ourselves. Calendar did when it was instantiated. Remember that 3rd week of September? Yup, it's here where it bites us back. It calculates the Sunday of the 3rd week in June and overwrites our day = 22 to the 15th, which is the 3rd Sunday in the week of June.

        if (isSet[DAY_OF_WEEK]) { //yup we set this
            int first = getFirstDayOfMonth(year, month);
    
            // 3: YEAR + MONTH + DAY_OF_WEEK_IN_MONTH + DAY_OF_WEEK
            if (isSet[DAY_OF_WEEK_IN_MONTH]) { //we didn't set this, but it's been set!
                if (fields[DAY_OF_WEEK_IN_MONTH] < 0) { 
                    month++;
                    first = getFirstDayOfMonth(year, month);
                    day = 1 + 7 * (fields[DAY_OF_WEEK_IN_MONTH]);
                } else
                    day = 1 + 7 * (fields[DAY_OF_WEEK_IN_MONTH] - 1); 
                     // 15 = 1 + 7 * (3 - 1)
    
                int offs = fields[DAY_OF_WEEK] - first;
                if (offs < 0)
                    offs += 7;
                day += offs;
            }
        }
    

    How We Can Solve the Problem

    Check the source javadoc on the set method you called with year, month, and day.

    Sets the values for the calendar fields YEAR, MONTH, DAY_OF_MONTH, HOUR_OF_DAY, and MINUTE. Previous values of other fields are retained. If this is not desired, call clear() first.

    Sure enough, adding dc.clear(); before your set returns the desired result. I didn't walk through it, but given the weird behavior currently experienced the best bet would be to getTime() after each change will make sure your time doesn't become an unexpected result.

    So basically, what was causing the problem was that you had half-dirty data with half-new data that contradicted itself and overwrote things. If this code had been run next week, you wouldn't have even caught the bug. How's that for an inconvenient time-dependent condition?