TOP

このエントリーをはてなブックマークに追加

Temporary Field

Temporary Field

兆候と症状
Temporary fieldsは、特定の状況下でのみ値を取得します（したがって、オブジェクトに必要です）。これらの状況以外では、それらは空です。

※Temporary Fieldに関してはこの記事の方が私はわかりやすかったので、兆候と症状のところはこの記事をベースに書いてみます。

public class Estimator {
  private readonly TimeSpan defaultEstimate;
  private IReadOnlyCollection<TimeSpan> durations;
  private TimeSpan average;
  private TimeSpan standardDeviation;

  public Estimator(TimeSpan defaultEstimate) {
      this.defaultEstimate = defaultEstimate;
  }

  public TimeSpan CalculateEstimate(
    IReadOnlyCollection<TimeSpan> durations) {
    if (durations == null)
      throw new ArgumentNullException(nameof(durations));

    if (durations.Count == 0)
      return this.defaultEstimate;

    this.durations = durations;
    this.CalculateAverage();
    this.CalculateStandardDeviation();

    var margin = TimeSpan.FromTicks(this.standardDeviation.Ticks * 3);
    return this.average + margin;
  }

  private void CalculateAverage() {
    this.average =
        TimeSpan.FromTicks(
            (long)this.durations.Average(ts => ts.Ticks));
  }

  private void CalculateStandardDeviation() {
    var variance =
        this.durations.Average(ts => 
            Math.Pow(
                (ts - this.average).Ticks,
                2));
    this.standardDeviation = 
        TimeSpan.FromTicks((long)Math.Sqrt(variance));
  }
}

上記EstimatorクラスのCalculateEstimateメソッド内でdurationsというフィールドを使っています。同時に、CalculateAverageメソッド内ではaverageを使っており、CalculateStandardDeviationメソッド内ではstandardDeviationを使っています。これらのメソッドはCalculateEstimateメソッドで呼ばれているため、CalculateEstimateは、明示的にdurationsを使い、さらに暗黙的にaverage, standardDeviationフィールドも使っています。 averageとstandardDeviationはdurationsに依存しています。さらに、standardDeviationはaverageに依存しています。 ここで、CalculateEstimateメソッド内の処理の順番を以下のように変更したらどうなるでしょうか？

this.durations = durations;
this.CalculateAverage();
this.CalculateStandardDeviation();
this.durations = durations;
this.CalculateStandardDeviation();
this.CalculateAverage();

コンパイルは通ります。CalculateEstimateを実行してもExceptionは発生しません。しかし、想定外の結果になります。 このコードは理解しにくいだけではなく、脆弱です（処理の順番を入れ替えただけで破綻します）。さらに、スレッドセーフでもありません。

問題の理由
多くの場合、Temporary fieldsは、大量の入力を必要とするアルゴリズムで使用するために作成されます。そのため、プログラマーはメソッド内に多数のパラメーターを作成する代わりに、クラスの中にこのデータのフィールドを作成することにします。これらのフィールドはアルゴリズムでのみ使用され、それ以外では使用されません。この種のコードは理解するのが難しいです。オブジェクトフィールドにデータが表示されるはずですが、何らかの理由でほとんど常に空です。（フィールドに値を設定するメソッドが実行されるまではnullが設定されている）

対処
Temporary fieldとそれらを操作するすべてのコードは、「Extract Class」を介して別のクラスに入れることができます。つまり、メソッドオブジェクトを作成して、「Replace Method with Method Object」を実施します。

「Introduce Null Object」を導入し、Temporary fieldの値の存在を確認するために使われた条件コードをそこに統合します。

効果

  • コードが明確になり、コードの構成が改善されます。


書籍としてはこの辺りが参考になると思います。

リファクタリング第2版
refactoring

レガシーコード改善ガイド
legacy_code



↑