2

I'm troubleshooting an issue with converting a GregorianCalendar that only represents the current date (ie// 2013-03-10 00:00:00) to a java.util.Date object. The idea behind this test is to take two dates - one with only the current date, and one with only the current time (ie// 1970-01-01 12:30:45), and combine them into one date representing the Date and Time (2013-03-10 12:30:45).

On the day when the DST switch occured, the test failed - because converting the GregorianCalendar to a date object (Date date = dateCal.getTime(); in the code below) lost an hour and thus rolled back to (2013-03-09 23:00:00). How can I make this not happen?

public static Date addTimeToDate(Date date, Date time) {
    if (date == null) {
        throw new IllegalArgumentException("date cannot be null");
    } else if (time == null) {
        throw new IllegalArgumentException("time cannot be null");
    } else {
        Calendar timeCal = GregorianCalendar.getInstance();
        timeCal.setTime(time);

        long timeMs = timeCal.getTimeInMillis() + timeCal.get(Calendar.ZONE_OFFSET) + timeCal.get(Calendar.DST_OFFSET);
        return addMillisecondsToDate(date, timeMs);
    }
}


@Test
public void testAddTimeToDate() {
    Calendar expectedCal = Calendar.getInstance();
    Calendar dateCal = Calendar.getInstance();
    dateCal.clear();
    dateCal.set(expectedCal.get(Calendar.YEAR), expectedCal.get(Calendar.MONTH), expectedCal.get(Calendar.DAY_OF_MONTH));

    Calendar timeCal = Calendar.getInstance();
    timeCal.clear();
    timeCal.set(Calendar.HOUR_OF_DAY, expectedCal.get(Calendar.HOUR_OF_DAY));
    timeCal.set(Calendar.MINUTE, expectedCal.get(Calendar.MINUTE));
    timeCal.set(Calendar.SECOND, expectedCal.get(Calendar.SECOND));
    timeCal.set(Calendar.MILLISECOND, expectedCal.get(Calendar.MILLISECOND));

    Date expectedDate = expectedCal.getTime();
    Date date = dateCal.getTime();
    Date time = timeCal.getTime();

    Date actualDate = DateUtil.addTimeToDate(date, time);

    assertEquals(expectedDate, actualDate);
}
tamuren
  • 1,072
  • 4
  • 15
  • 32
  • 1
    `Calendar.getInstance(Locale.FRANCE).getTime()` without using the gregorian date is not feasible? – Joop Eggen Mar 14 '13 at 14:18
  • Why not clearing the time fields of `dateCal` (as you already did) and after that set the time fields to those of `timeCal` instead of `DateUtil.addTimeToDate`? That is: use `Calendar` instead of `Date`. – Claude Mar 14 '13 at 14:27
  • BTW: I guess `assertEquals` will sometimes or often fail because of the elapsed time between your calls to `Calendar.getInstance()`. – Claude Mar 14 '13 at 14:29
  • @Michael: The Calendar objects are used to create specific date objects we want for this test, with the purpose of ensuring the functionality of DateUtil.addTimeToDate(). Ignoring DateUtil is outside the scope of this test. – tamuren Mar 14 '13 at 14:43
  • @JoopEggen: Your suggestion is only not feasible because I do not know what you mean. Can you please provide an example? – tamuren Mar 14 '13 at 14:45
  • If this is actually a test for `DateUtil` then it seems behaviour on the day of DST changing is actual a test case on its own. – Claude Mar 15 '13 at 08:48

4 Answers4

0

Why are you including the timezone offsets in your calculation? when you are working with milliseconds in Java, they are always in UTC. you don't need to do any additional conversions.

Your biggest problem is probably trying to do these date/time calculations manually. you should be using the Calendar class itself to handle the calculations.

jtahlborn
  • 52,909
  • 5
  • 76
  • 118
  • Do you think the Calendar class can entirely replace the DateUtil method? I'd like to know how, but if that's too much a simple true/false will have me looking much closer at the Calendar class. – tamuren Mar 14 '13 at 14:53
  • @tamuren - yes, this is the whole _point_ of the class, e.g. see http://docs.oracle.com/javase/6/docs/api/java/util/Calendar.html#add%28int,%20int%29 – jtahlborn Mar 14 '13 at 15:25
  • I tried using Calendar for this purpose, but simply adding the two calendars together did not compensate for the change in zone_offset so the result was still off by 1 hour – tamuren Mar 19 '13 at 17:41
0

I tried and did not get a difference. Even varied locale, and substituted GregorianCalendar with Calendar..

Used:

private static Date addMillisecondsToDate(Date date, long timeMs) {
    return new Date(date.getTime() + timeMs);
}

The upcoming Java 8 has better date/time support.

Joop Eggen
  • 107,315
  • 7
  • 83
  • 138
  • If you use a Date representing March 10th at midnight, a date representing any time from epoch past 2AM, then the result is 1 hour greater than expected. Since both Dates have an offset of -8 (if your PST), then when they're added together the offset is -7 (since you're now in PDT) and the time is an hour greater than desired – tamuren Mar 19 '13 at 17:34
0

This is how I ended up refactoring my method to compensate for the lost / gained hour due to DST: 

public static Date addTimeToDate(Date date, Date time) {
    if (date == null) {
        throw new IllegalArgumentException("date cannot be null");
    } else if (time == null) {
        throw new IllegalArgumentException("time cannot be null");
    } else {
        Calendar dateCal = GregorianCalendar.getInstance();
        dateCal.setTime(date);

        Calendar timeCal = GregorianCalendar.getInstance();
        timeCal.setTime(time);
        int zoneOffset = timeCal.get(Calendar.ZONE_OFFSET);

        if (dateCal.get(Calendar.MONTH) == Calendar.MARCH) {
            if (Calendar.SUNDAY == dateCal.get(Calendar.DAY_OF_WEEK) && dateCal.get(Calendar.DAY_OF_MONTH) >= 7
                    && dateCal.get(Calendar.DAY_OF_MONTH) <= 14 && timeCal.get(Calendar.HOUR_OF_DAY) >= 3) {
                zoneOffset -= TimeUnit.MILLISECONDS.convert(1, TimeUnit.HOURS);
            }
        } else if (dateCal.get(Calendar.MONTH) == Calendar.NOVEMBER) {
            if (Calendar.SUNDAY == dateCal.get(Calendar.DAY_OF_WEEK) && dateCal.get(Calendar.DAY_OF_MONTH) <= 7
                    && timeCal.get(Calendar.HOUR_OF_DAY) >= 3) {
                zoneOffset += TimeUnit.MILLISECONDS.convert(1, TimeUnit.HOURS);
            }
        }
        long timeMs = timeCal.getTimeInMillis() + zoneOffset + timeCal.get(Calendar.DST_OFFSET);
        return addMillisecondsToDate(date, timeMs);
    }
}

I'm not fond of this method because if the rules for DST ever change then this method will need to be updated. Is there a library that would perform a similar function ?

tamuren
  • 1,072
  • 4
  • 15
  • 32
0

tl;dr

ZonedDateTime.of(
    LocalDate.parse( "2013-03-10" ) ,
    LocalTime.parse( "12:30:45" ) ,
    ZoneId.of( "Africa/Tunis" )
)                                    // Instantiate a `ZonedDateTime` object.
.toString()                          // Moment seen through wall-clock time of people in Tunisia time zone.

2013-03-10T12:30:45+01:00[Africa/Tunis]

ZonedDateTime.of(
    LocalDate.parse( "2013-03-10" ) ,
    LocalTime.parse( "12:30:45" ) ,
    ZoneId.of( "Africa/Tunis" )
)
.toInstant()                         // Convert to `Instant` from `ZonedDateTime`, for UTC value.
.toString()                          // Same moment, adjusted into wall-clock time of UTC. The Tunisian wall-clock is an hour ahead of UTC, but both represent the same simultaneous moment, same point on the timeline.

2013-03-10T11:30:45Z

UTC versus Zoned

converting the GregorianCalendar to a date object … lost an hour and thus rolled back

A GregorianCalendar includes a time zone. If you do not specify a time zone, the JVM’s current default time zone is implicitly assigned. A java.util.Date, in contrast, is always in UTC. Confusingly, the Date::toString method dynamically assigns the JVM’s current default time zone while generating the string, creating the illusion of an assigned time zone when in fact the internal value is UTC. An awful confounding mess.

We cannot further diagnose your specifics because you did not provide information about the time zones involved on your machine.

But this is all moot, as you should be using the java.time classes instead.

Avoid legacy date-time classes

You are using troublesome old date-time classes that are now legacy, supplanted by the modern java.time classes.

java.time

The idea behind this test is to take two dates - one with only the current date, and one with only the current time (ie// 1970-01-01 12:30:45), and combine them into one date representing the Date and Time (2013-03-10 12:30:45).

For a time-of-day, use LocalTime. For a date-only, use LocalDate. 

LocalDate ld = LocalDate.parse( "2013-03-10" ) ;
LocalTime lt = LocalTime.parse( "12:30:45" ) ;

Neither of those have a time zone, nor offset-from-UTC. So they have no meaning until assigned a zone or offset.

Specify a proper time zone name in the format of continent/region, such as America/Montreal, Africa/Casablanca, or Pacific/Auckland. Never use the 3-4 letter abbreviation such as EST or IST as they are not true time zones, not standardized, and not even unique(!). 

ZoneId z = ZoneId.of( "Pacific/Auckland" ) ;

Assign the zone to the date and time to get a ZonedDateTime. 

ZonedDateTime zdt = ZonedDateTime.of( ld , lt , z ) ;

Now we have an actual moment, a point on the timeline. The ZonedDateTime class adjusts your time-of-day if your passed LocalTime is not valid on that particular date in that zone. Such an adjustment is needed in case of an anomaly such as Daylight Saving Time (DST). Be sure to read the doc to understand the algorithm of that adjustment, to see if you agree its approach.

To see that same moment in UTC, extract an Instant. Same point on the timeline, different wall-clock time.

Instant instant = zdt.toInstant() ;

About java.time

The java.time framework is built into Java 8 and later. These classes supplant the troublesome old legacy date-time classes such as java.util.Date, Calendar, & SimpleDateFormat.

The Joda-Time project, now in maintenance mode, advises migration to the java.time classes.

To learn more, see the Oracle Tutorial. And search Stack Overflow for many examples and explanations. Specification is JSR 310.

Where to obtain the java.time classes?

The ThreeTen-Extra project extends java.time with additional classes. This project is a proving ground for possible future additions to java.time. You may find some useful classes here such as Interval, YearWeek, YearQuarter, and more.

Basil Bourque
  • 303,325
  • 100
  • 852
  • 1,154