Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

java calendar returns wrong week

Tags:

java

calendar

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.

like image 289
Ioan-Cristian Linte Avatar asked Sep 18 '14 16:09

Ioan-Cristian Linte


1 Answers

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?

like image 143
Compass Avatar answered Sep 21 '22 09:09

Compass