0
Эти методы смехотворно глупы, ИМО, но я хочу понять, что другие разработчики думают о таком коде. Критика может включать технические и стилистические ошибки. Исправления могут использовать что-либо из Apache commons-lang, например StringUtils, DateUtils и т. Д., А также что-либо в Java 5. Код предназначен для веб-приложения, если это повлияет на ваш стиль. Эти четыре метода все определены в том же файле, если это имеет значение. Я упоминал, что для этого кода нет модульных тестов? Что бы вы сделали, чтобы исправить ситуацию? Я только что закончил этот файл, и это не моя непосредственная задача, чтобы исправить этот код. Я мог бы в свободное время, если захочу.Сколько проблем возникает в следующих процедурах синтаксического анализа даты, которые поступают из реального проекта?
Метод один:
public static boolean isFromDateBeforeOrSameAsToDate(final String fromDate,
final String toDate) {
boolean isFromDateBeforeOrSameAsToDate = false;
Date fromDt = null;
Date toDt = null;
try {
fromDt = CoreUtils.parseTime(fromDate, CoreConstants.DATE_PARSER);
toDt = CoreUtils.parseTime(toDate, CoreConstants.DATE_PARSER);
// if the FROM date is same as the TO date - its OK
// if the FROM date is before the TO date - its OK
if (fromDt.before(toDt) || fromDt.equals(toDt)) {
isFromDateBeforeOrSameAsToDate = true;
}
} catch (ParseException e) {
e.printStackTrace();
}
return isFromDateBeforeOrSameAsToDate;
}
Метод два:
public static boolean isDateSameAsToday(final Date date) {
boolean isSameAsToday = false;
if (date != null) {
Calendar current = Calendar.getInstance();
Calendar compare = Calendar.getInstance();
compare.setTime(date);
if ((current.get(Calendar.DATE) == compare.get(Calendar.DATE))
&& (current.get(Calendar.MONTH) == compare
.get(Calendar.MONTH))
&& (current.get(Calendar.YEAR) == compare
.get(Calendar.YEAR))) {
isSameAsToday = true;
}
}
return isSameAsToday;
}
Способ третий:
public static boolean areDatesSame(final String fromDate,
final String toDate) {
boolean areDatesSame = false;
Date fromDt = null;
Date toDt = null;
try {
if (fromDate.length() > 0) {
fromDt = CoreUtils.parseTime(fromDate,
CoreConstants.DATE_PARSER);
}
if (toDate.length() > 0) {
toDt = CoreUtils.parseTime(toDate, CoreConstants.DATE_PARSER);
}
if (fromDt != null && toDt != null) {
if (fromDt.equals(toDt)) {
areDatesSame = true;
}
}
} catch (ParseException e) {
if (logger.isDebugEnabled()) {
e.printStackTrace();
}
}
return areDatesSame;
}
Метод четыре:
public static boolean isDateCurrentOrInThePast(final Date compareDate) {
boolean isDateCurrentOrInThePast = false;
if (compareDate != null) {
Calendar current = Calendar.getInstance();
Calendar compare = Calendar.getInstance();
compare.setTime(compareDate);
if (current.get(Calendar.YEAR) > compare.get(Calendar.YEAR)) {
isDateCurrentOrInThePast = true;
}
if (current.get(Calendar.YEAR) == compare.get(Calendar.YEAR)) {
if (current.get(Calendar.MONTH) > compare.get(Calendar.MONTH)) {
isDateCurrentOrInThePast = true;
}
}
if (current.get(Calendar.YEAR) == compare.get(Calendar.YEAR)) {
if (current.get(Calendar.MONTH) == compare.get(Calendar.MONTH)) {
if (current.get(Calendar.DATE) >= compare
.get(Calendar.DATE)) {
isDateCurrentOrInThePast = true;
}
}
}
}
return isDateCurrentOrInThePast;
}
Вот как я хотел бы написать одно и то же (ну, сначала я бы написал модульные тесты, но я пропущу это здесь).
public static int compareDatesByField(final Date firstDate,
final Date secondDate, final int field) {
return DateUtils.truncate(firstDate, field).compareTo(
DateUtils.truncate(secondDate, field));
}
public static int compareDatesByDate(final Date firstDate,
final Date secondDate) {
return compareDatesByField(firstDate, secondDate, Calendar.DATE);
}
// etc. as required, although I prefer not bloating classes which little
// methods that add little value ...
// e.g., the following methods are of dubious value, depending on taste
public static boolean lessThan(int compareToResult) {
return compareToResut < 0;
}
public static boolean equalTo(int compareToResult) {
return compareToResut == 0;
}
public static boolean greaterThan(int compareToResult) {
return compareToResut > 0;
}
public static boolean lessThanOrEqualTo(int compareToResult) {
return compareToResut <= 0;
}
public static boolean greaterThanOrEqualTo(int compareToResult) {
return compareToResut >= 0;
}
// time-semantic versions of the dubious methods - perhaps these go in TimeUtils ?
public static boolean before(int compareToResult) {
return compareToResut < 0;
}
public static boolean on(int compareToResult) {
return compareToResut == 0;
}
public static boolean after(int compareToResult) {
return compareToResut > 0;
}
public static boolean onOrBefore(int compareToResult) {
return compareToResut <= 0;
}
public static boolean onOrAfter(int compareToResult) {
return compareToResut >= 0;
}
Клиенты могли бы использовать метод следующим образом:
/* note: Validate library from Apache Commons-Lang throws
* IllegalArgumentException when arguments are not valid
* (this comment would not accompany actual code since the
* Javadoc for Validate would explain that for those unfamiliar with it)
*/
Validate.isTrue(onOrAfter(compareDatesByDate(registrationDate, desiredEventDate),
"desiredEventDate must be on or after the *day* of registration: ", desiredEventDate);
Таким образом, одна техническая ошибка, которая может быть не очевидна из приведенного выше примера кода, заключается в том, что она использует статический экземпляр DateFormat для разбора дат String! DateFormat не является потокобезопасным без правильной синхронизации, поэтому код * неправильный * в объективном смысле, а также стилистически хромой! – les2
Обработка исключений является острой! Мы используем log4j, поэтому a if (logger.isDebugEnabled()) {logger.debug ("не удалось разобрать дату" + dateString + "'"); } будет лучше, чем e.printStackTrace() ;, если вообще требуется ведение журнала - может быть достаточно простую трассировку стека «// swallow stack, поскольку мы игнорируем ошибки синтаксического анализа, возвращая null или false c» комментарий в блоке catch. – les2
LES2 - не видя, как они используют статический DateFormat, трудно сказать, не является ли это потокобезопасным.Операция синтаксического анализа строки в дату не должна влиять на внутреннее состояние любого объекта DateFormat. Если, однако, они корректируют календарь, связанный с DataFormat, тогда возникает проблема. Почему Sun сделал DateFormat изменчивым по этим небольшим параметрам, находится вне меня. В любом случае, так как большинство людей используют DateFormat, я уверен, что безопасно иметь статический экземпляр. –