CodeReview: Разница в датах Java (в разрешении дня) - PullRequest
Новая
       12

CodeReview: Разница в датах Java (в разрешении дня)

2 голосов
/ 19 мая 2009

Пожалуйста, оставьте свой отзыв о следующем коде.

Мне нужно рассчитать разницу в днях между двумя объектами Date. Гарантируется, что оба объекта Date находятся в одной и той же временной зоне. 

public class DateUtils {
public final static long DAY_TIME_IN_MILLIS = 24 * 60 * 60 * 1000;

/**
 * Compare between 2 dates in day resolution.
 * 
 * @return positive integer if date1 > date2, negative if date1 < date2. 0 if they are equal.
 */
public static int datesDiffInDays(final Date date1, final Date date2){
    long date1DaysMS = date1.getTime() - (date1.getTime() % DAY_TIME_IN_MILLIS);
    long date2DaysMS = date2.getTime() - (date2.getTime() % DAY_TIME_IN_MILLIS);

    long timeInMillisDiff = (date1DaysMS - date2DaysMS);
    int ret = (int) (timeInMillisDiff / DAY_TIME_IN_MILLIS); 
    return ret;
}

Можете ли вы указать на проблему, которую я мог пропустить?

РЕДАКТИРОВАТЬ: @mmyers спросил, если пройти мой модульный тест. Ну да. Но у меня нет никакого реального опыта с датами, и я знаю, что это большая тема. Ниже приведен юнит-тест, который я использую. 

public class TestMLDateUtils {

@Test
public final void testDatesDiffInDays() {
    TimeZone.setDefault(TimeZone.getTimeZone("UTC"));

    // 00:00:00.000 1.1.1970
    Calendar cal1970 = Calendar.getInstance();
    cal1970.setTimeInMillis(0);

    Calendar tested = Calendar.getInstance();
    tested.setTimeInMillis(0);

    // Add 1 millisecond, date = 00:00:00.001 1.1.1970
    tested.add(Calendar.MILLISECOND, 1);

    assertTrue(DateUtils.datesDiffInDays(cal1970.getTime(), tested.getTime()) == 0);

    // Add 1 second, date = 00:00:01.001 1.1.1970
    tested.add(Calendar.SECOND, 1);
    assertTrue(DateUtils.datesDiffInDays(cal1970.getTime(), tested.getTime()) == 0);

    // Add 1 minute, date = 00:01:01.001 1.1.1970
    tested.add(Calendar.MINUTE, 1);
    assertTrue(DateUtils.datesDiffInDays(cal1970.getTime(), tested.getTime()) == 0);

    // Add 1 hour, date = 01:01:01.001 1.1.1970
    tested.add(Calendar.HOUR_OF_DAY, 1);
    assertTrue(DateUtils.datesDiffInDays(cal1970.getTime(), tested.getTime()) == 0);

    // date = 23:59:59.999 1.1.1970
    tested.setTimeInMillis(0);
    tested.add(Calendar.MILLISECOND, 999);
    tested.add(Calendar.SECOND, 59);
    tested.add(Calendar.MINUTE, 59);
    tested.add(Calendar.HOUR_OF_DAY, 23);
    //System.out.println("D: " + tested.getTime());
    assertTrue(DateUtils.datesDiffInDays(cal1970.getTime(), tested.getTime()) == 0);

    // date = 00:00:00.000 2.1.1970
    tested.setTimeInMillis(0);
    tested.add(Calendar.DAY_OF_MONTH, 1);
    assertTrue(DateUtils.datesDiffInDays(cal1970.getTime(), tested.getTime()) == -1);
    assertTrue(DateUtils.datesDiffInDays(tested.getTime(), cal1970.getTime()) == 1);

    // date = 00:00:00.001 2.1.1970
    tested.add(Calendar.MILLISECOND, 1);
    assertTrue(DateUtils.datesDiffInDays(cal1970.getTime(), tested.getTime()) == -1);
    assertTrue(DateUtils.datesDiffInDays(tested.getTime(), cal1970.getTime()) == 1);

    // date = 00:00:01.001 2.1.1970
    tested.add(Calendar.SECOND, 1);
    assertTrue(DateUtils.datesDiffInDays(cal1970.getTime(), tested.getTime()) == -1);
    assertTrue(DateUtils.datesDiffInDays(tested.getTime(), cal1970.getTime()) == 1);

    // date = 00:01:01.001 2.1.1970
    tested.add(Calendar.MINUTE, 1);
    assertTrue(DateUtils.datesDiffInDays(cal1970.getTime(), tested.getTime()) == -1);
    assertTrue(DateUtils.datesDiffInDays(tested.getTime(), cal1970.getTime()) == 1);

    // date = 01:01:01.001 2.1.1970
    tested.add(Calendar.HOUR_OF_DAY, 1);
    assertTrue(DateUtils.datesDiffInDays(cal1970.getTime(), tested.getTime()) == -1);
    assertTrue(DateUtils.datesDiffInDays(tested.getTime(), cal1970.getTime()) == 1);

    // date = 13:01:01.001 2.1.1970
    tested.add(Calendar.HOUR_OF_DAY, 12);
    assertTrue(DateUtils.datesDiffInDays(cal1970.getTime(), tested.getTime()) == -1);
    assertTrue(DateUtils.datesDiffInDays(tested.getTime(), cal1970.getTime()) == 1);
}
}

Ответы [ 5 ]

8 голосов
/ 19 мая 2009

  • Непосредственная проблема: дни могут иметь меньше или больше 24 часов из-за изменения летнего времени.

  • Вторичная проблема: обычно, когда люди думают в днях, они действительно означают «человеческие дни», а не «24-часовые периоды». Другими словами, многие люди сказали бы, что 7 часов вечера 7 утра следующего дня - это разница в день, тогда как 7 часов утра 7 вечера в тот же день - ноль дней. Оба 12 часов. В этот момент вам действительно нужно знать календарь, который рассматривается.

Конечно, это может не иметь значения для вашей ситуации, но мы на самом деле не знаем, что это такое.

  • Третья проблема: вы используете встроенный календарь API вместо Joda Time . Это почти никогда не хорошая идея - это ужасно и пронизано ошибками и проблемами. И да, постоянные посетители здесь скажут вам, что всегда часть моего ответа, когда дело доходит до дат и времени Java - и по уважительной причине. Это действительно так важно.

РЕДАКТИРОВАТЬ: ваш тест устанавливает часовой пояс по умолчанию, чтобы быть UTC. Это не очень хорошая идея (особенно без сброса в утверждении finally). Часовые пояса сложны, но вы должны подумать о том, какие у вас есть значения, что они значат и какие часовые пояса задействованы.

1 голос
/ 19 мая 2009

Есть много измерений для обзора кода; вместо правильности, к которой обращаются другие, позвольте мне немного сосредоточиться на стиле. Это, конечно, будет несколько более субъективным, чем обзор, сосредоточенный на правильности.

Я бы включил переменную ret. Это увеличивает размер метода без улучшения читабельности.

Я хотел бы рассмотреть разделение преобразования между миллисекундами и днями на отдельную функцию. Ваш полный класс, вероятно, выполняет это разделение в нескольких местах. Даже если это не так, полезно то, что проще называть функции, которые выполняют только одно действие.

Говоря о наименовании, я бы переименовал функцию, возможно, в "dayDifference" - сокращения вызывают много проблем, не в последнюю очередь из-за сложности запоминания, какое сокращение использовалось при каких обстоятельствах. Если вы никогда не используете ничего, этот конкретный источник путаницы устраняется. Точно так же я бы переименовал константу в MILLISECONDS_PER_DAY.

1 голос
/ 19 мая 2009

Другая проблема, которая еще не упоминалась, это високосные секунды . Дни могут иметь больше или меньше 24 * 60 * 60 секунд из-за настроек времени UTC, чтобы поддерживать его более или менее в соответствии со средним солнечным годом. Вероятно, не имеет большого значения для вашего использования, но вы должны по крайней мере знать о возможности.

Хороший API - это то, что вам нужно, если у вас есть нетривиальные требования для работы с датами и временем. Ссылка на Joda Time в ответе Джона Скита, похоже, не работает, поэтому вот ссылка , которая работает.

1 голос
/ 19 мая 2009

Часовой пояс, если таковой имеется, в объекте Date не имеет значения, поскольку вы используете getTime(); что «[r] повторяет число миллисекунд с 1 января 1970 года, 00:00:00 по Гринвичу, представленное этим объектом Date.»

Однако вы не учитываете дополнительные секунды, которые могут возвращать некоторые реализации. Таким образом, если диапазон дня, который вы задаете, содержит одну или несколько дополнительных секунд, а ваше время достаточно близко к одному и тому же времени дня, ваш расчет может быть неправильным по дням. Тем не менее, по-видимому, нет никакого способа увидеть, объясняет ли какая-то конкретная реализация високосные секунды или нет (я полагаю, что большинство этого не делает), и разница в любом случае довольно банальна.

0 голосов
/ 19 мая 2009

Дата уже имеет этот метод, найдите Date.compareTo (Date) в Javadoc.

...