views:

57

answers:

3

Hello everybody,

Take a look at the piece of code bellow:

Calendar today1 = Calendar.getInstance();
today1.set(Calendar.DAY_OF_WEEK, Calendar.FRIDAY);
System.out.println(today1.getTime());

Calendar today2 = new GregorianCalendar(2010, Calendar.JULY, 14);
today2.set(Calendar.DAY_OF_WEEK, Calendar.FRIDAY);
System.out.println(today2.getTime());

I'm quite confused... Assuming I am running it today as July 14th, 2010, the output is:

Fri Jul 16 14:23:23 PDT 2010
Wed Jul 14 00:00:00 PDT 2010

The most annoying thing is that if I add today2.getTimeInMillis() (or any other get() method) it will produce consistent result. For the code bellow:

Calendar today2 = new GregorianCalendar(2010, Calendar.JULY, 14);
today2.getTimeInMillis();
today2.set(Calendar.DAY_OF_WEEK, Calendar.FRIDAY);
System.out.println(today2.getTime());

The result is:

Fri Jul 16 00:00:00 PDT 2010
+1  A: 

You should in fact be using Calendar#getInstance() to get an instance and not new GregorianCalendar(). Replace that line by

Calendar today2 = Calendar.getInstance();
today2.set(2010, Calendar.JULY, 14);

and it will go well.

Sorry, no detailed explanation for the behaviour, expect that Calendar along with java.util.Date are one of the major epic failures in the current Java SE API. If you're doing intensive date/time operations, then I'd recommend to have a look at JodaTime. The upcoming new Java 7 will ship with an improved date/time API based on JodaTime (JSR-310).

BalusC
+2  A: 

The answer is actually documented in the JavaDoc for java.util.Calendar

Quoted here:

set(f, value) changes calendar field f to value. In addition, it sets an internal member variable to indicate that calendar field f has been changed. Although field f is changed immediately, the calendar's milliseconds is not recomputed until the next call to get(), getTime(), or getTimeInMillis() is made.

So that explains the behavior you are seeing, but I concur with another responder to your question that you should consider JodaTime if you're going to do a lot of Date coding.

dustmachine
`Calendar` is indeed lazily loaded, but this doesn't *actually* explain the problem. Why doesn't it work with `new GregorianCalendar(y, m, d)` but does it work with `set(y, m, d)`?
BalusC
Yes, it does explain.It's because with GregorianCalendar(y, m, d) you have no WEEK_OF_YEAR computed yet. While with set(y, m, d) you have first called Calendar.getInstance() which gives us an object with all fields filled up.
Leo Holanda
A: 

(Sorry for the edit, I wanted this to be a little more readable, but couldn't get it right when I originally wrote the answer...now it's essay length, but there you go...)

Just to add to what's already been said, the issue arises from the returned Calendar instances being prepared differently. I personally feel like this is a design flaw, but there may be good reason for it.

When you call Calendar.getInstance(), it creates a new GregorianCalendar using the default constructor. This constructor calls setCurrentTimeMillis(time) with the current system time, and then calls the protected method complete().

However, when you create a new GregorianCalendar using the constructor that you did, complete() is never called; instead, among other things, only set(field, value) is called for the various bits of information that is provided. This is all well and good, but it has some confusing consequences.

When complete() is called in the first case, the member variables dustmachine alluded to are checked to determine what information should be recalculated. This results in a branch that forces calculation all of the fields (DAY, WEEK_OF_MONTH, etc.). Note that Calendar is indeed lazy; it just happens that using this method of instantiation forces an explicit recalculation (or in this case initial calculation) on the spot.

So, what impact does this have? Given that no upfront field computation was performed in the case of the second object creation, the two objects have vastly different states. The first has all of its field information populated, while the second only has the information which you provided. When you call the various get*() methods, it shouldn't matter, because any changes should provoke the lazy recalculation step when you retrieve the information. However, the order in which this recalculation occurs exposes the differences between the two varying initial states.

In your particular case, this is due to the following relevant code in computeTime(), which is necessarily invoked to compute the correct time when you request it with getTime():

boolean weekMonthSet = isSet[WEEK_OF_MONTH] || isSet[DAY_OF_WEEK_IN_MONTH];
...
boolean useDate = isSet[DATE];

if (useDate && (lastDateFieldSet == DAY_OF_WEEK
      || lastDateFieldSet == WEEK_OF_MONTH
      || lastDateFieldSet == DAY_OF_WEEK_IN_MONTH)) {
    useDate = !(isSet[DAY_OF_WEEK] && weekMonthSet);
}

In the first case, all fields are set due to that initial calculation. This allows weekMonthSet to be true, which, along with the DAY_OF_WEEK that you provided in your call to set(field, value) being set, causes useDate to be false.

However, in the second case, as no fields have been calculated, the only fields set are the ones you provided in the constructor and in the subsequent set(field, value) call. Thus, useDate will remain true, because isSet[DATE] is true per your constructor, but weekMonthSet is false as the other fields in the object have not been computed anywhere, nor set by you.

When useDate is true, as implied, it uses your date information to generate the value for the time. When useDate is false, it's able to use your DAY_OF_WEEK information to compute the time you expect, resulting in the difference you see.

Finally, this raises the question of why calling getTimeInMillis() before calling getTime() will fix the unexpected behaviour. As it turns out, the fields will be recalculated as a result of your set(field, value) call in both objects. This just happens to occur after the time is calculated, for whatever (probably genuine) reason. Therefore, forcing the time to be calculated once on the second Calendar will essentially align the states of the two objects. After that, I believe the calls to get*() should all work consistently for both objects.

Ideally, the constructor you used in the second case should perform this initial calculation step in the name of consistency (although maybe for reasons of performance this wouldn't be preferred), but it doesn't, and this is what you get.

So, in short, as the others mentioned, JodaTime is your friend, and clearly these classes are less so. :)

Tim Stone