0

これらのメソッドはばかばかしいほどばかげていますが、私は他の開発者がそのようなコードについてどう思うかを知りたいと思っています。批判には、技術的および文体上のエラーが含まれる場合があります。修正には、StringUtils、DateUtils などの Apache commons-lang からのもの、および Java 5 のものを使用できます。コードは、スタイルに影響する場合、Web アプリケーションを対象としています。これら 4 つのメソッドもすべて同じファイルで定義されています。このコードには単体テストもないと言いましたか?! 状況を修正するために何をしますか? たまたまこのファイルを見つけたので、このコードを修正するのは私の当面の仕事ではありません。必要に応じて、空き時間にできます。

方法 1:

   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;
    }

方法 2:

    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;
    }

方法 3:

    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;
    }

方法 4:

    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);
4

2 に答える 2

0

まず、インデントを修正します。Ctrl+ Shift+Fエクリプスで。

正しくインデントされていないコードは我慢できません。

次に、触れる前に、触れるすべてのメソッドの単体テストを作成します。

また、JodaTimeを使用します。これは、標準の Java 日付クラスを完全に打ち負かします。醜い日付ロジックの多くは、それに切り替えることで処理されます。

于 2009-12-18T16:05:33.760 に答える
0

ここにはあらゆる種類の問題があります。たとえば、なぜ静的メソッドなのですか? しかし、私の一番の問題はユニットテストの欠如です。ここで適用したいリファクタリングが何であれ、何かを壊していないことを確認するためのテストが必要です。

したがって、ユニットテストの作成から始めます。他のすべての問題はそれに次ぐものです。

于 2009-12-18T16:07:11.413 に答える