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

レガシーコードを改善する前に

遠回りな対応方法としてレビュー時に文句を言われると思う。

  • そこだけ変えても、そこ以外の糞コードはそのままだぞ。意味ないのでは?
  • 動くようになっている箇所の構成を帰るのはいかがなものか?

しかし、「これを続けていけば既存コードがテストで保護され安全に変更作業を行うことができるようになる。」という事実を説いて回る必要がある。
ここをクリアしないことには業務で公式に使用するのは難しい。

時間がないのに変更しなければなりません

緊急性の高いが規模の小さい仕変対応が突発的に発生したとする。
期限の関係から "テストコードを書かずに対応を行う。"、"テストコードを書いてから対応を行う。" の2択に迫られることがあると思う。
ここで後者の手順を選択するかどうかは、最終的に本人の気持ち次第になってしまう。
"あとでテストコード書く"という言い訳でやり過ごすことはできるが、"あとで"が来ることは無い。

→ テストコードを書いて助かったという経験が無いとなかなか難しそう。
TDDを実践しているプロジェクトに参画するとかトレーニングを受けるなどして経験値を得る必要がある。
自分で独学しても知識としては身につくが、なかなか実践するのが難しい。(実践していない案件でTDDを用いるには上の説得が必要だろう、
自宅で開発しているツールで用いでも基礎の基礎しか身につかないだろう。複雑な業務システムへの適用経験が欲しい。)

スプラウトメソッド

未整理のクラスに対して機能追加を行う際の手法
対象のクラスをテストコードコードに載せられるようにリファクタリングし、依存関係を取り除く。
1,対象のメソッドへ直接処理を書くのではなくて、新規メソッドを作成してそこに書く。
2,そのメソッドについてのテストコードを作成する。
3,そのメソッドを対象メソッドから呼び出す。

これによって、テスト済みコードと未テストコードが明確に分かれる。
メソッドが作成できない場合は、staticなクラスに処理を書く。

public class Entry
{
  public DateTime CreateAt { get; set; }
  public string Category { get; set; }
  public string Title { get; set; }
}

public class EntryManager
{
  public List<Entry> CurrentEntries { get; private set; }

  public EntryManager()
  {
    CurrentEntries = new List<Entry>();
  }

//オリジナル
public void PostEntries_Original(List<Entry> entries)
{
  //無条件にEntryを追加しているが "特定のカテゴリに属するものについては追加しない"という仕変に対応する場合を考える。
  //最も安直な対応としては、このメソッド内にチェック処理を追加するというものである。
  //2つの処理が混在することになる。また、テスト範囲が不明確になってしまう懸念がある。

  var createat = DateTime.Now;
  foreach (var entry in entries)
  {
    entry.CreateAt = createat;
  }

  CurrentEntries.Clear();
  CurrentEntries = entries;
}

//対応後
public void PostEntries_Updated(List<Entry> entries)
{
  //追加機能を別メソッドに配置し、それを呼び出すという仕変対応をとった。
  //その後、追加された機能であるIsNgCategory()、振る舞いが変更されたPostEntries_Updated()に対してテストコードを作成した。
  //
  //※この例では外部の依存性が存在していないが、実際ではまずはじめに依存性の排除を行なってからこれらの対応を行う必要があることに注意すべし。

  var createat = DateTime.Now;
  CurrentEntries.Clear();
  foreach (var entry in entries)
  {
    if (IsNgCategory(entry))
      continue;
    entry.CreateAt = createat;
    CurrentEntries.Add(entry);
  }
}

private bool IsNgCategory(Entry entry)
{
  return entry.Category == "NGカテゴリー";
}
}
スプラウトクラス

対象クラスの依存性分離を時間内に行えない場合(複雑すぎて困難)、
追加する機能・必要な既存機能を別クラスへ移動し、それを既存クラスから呼び出す手法を取る。
スプラウトクラスを作りすぎると構造がばらばらに散らばってしまうので多用には注意すべし。
※記載されている適用手順ではトップダウンなやり方であるが、個人的にボトムアップのほうが好き。

ラップメソッド

対象クラスに対して機能追加を行う際の手法2

//オリジナル
public class PaymentProcesser_Original
{
    //pay()呼び出しにロギング機能を追加したい。
    //使用側からはIFを帰ることなく、振る舞いを変更する場合にはラップメソッドを作成し、
    //そこに元処理を移動する方法もある。

    //元処理で様々な処理を行なっている場合、適切なラップメソッド名をつけるのが難しい・不適切な名前をつける場合があるので注意する。
    //その時はリファクタリングを行い適切な名前をつけれるように整理する。

    public void Pay()
    {
        //支払い処理
    }
}

//対応後
public class PaymentProcesser_Update
{
    private void _pay()
    {
        //支払い処理
    }

    private void WriteLog(string msg)
    {
        //ログ出力
    }

    public void Pay()
    {
        WriteLog("支払い処理実行");
        _pay();
    }
}

//対応後2
public class PaymentProcesser_Update2
{
    //既存機能をPublicにするのもあり。

    public void Pay()
    {
        //支払い処理
    }

    private void WriteLog(string msg)
    {
        //ログ出力
    }

    public void PayWithLogging()
    {
        WriteLog("支払い処理実行");
        Pay();
    }
}