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

このメソッドをテストハーネスで動かすことができません

隠れたメソッド

privateなメソッドをテストしたい場合の対処方法。
・publicなメソッドにしてしまう。*1
・そのメソッドの責務を別クラスに分離する。
テストクラスからそのメソッドを呼び出せるようにリファクタリングする。
→ publicにすることもできない。依存性が複雑に絡まっているのでクラスの分離も納期的に難しい。 場合の最終的な方法である。

※テストが行い易いように設計する。テストが容易な設計は良い設計である。

//「テストクラスからそのメソッドを呼び出せるようにリファクタリングする。」 サンプル

//元クラス
public class CCAImage
{
  //テスト対象なメソッド
  private void setSnapRegion(int x, int y, int dx, int dy){
    ...
  }

  public void snap(){
    ...
  }

  ...
}

//テストクラス
public class TestingCCAImage : CCAImage
{
  //元クラスのテスト対象メソッドをprotectedに変更し、それをオーバーライド。
  protected override void setSnapRegion(int x, int y, int dx, int dy){
    vase.setSnapRegion(...);
  }

  ...
}

言語の「便利」機能

「sealedなクラスが絡んでいるレガシーコードをテスト可能にする」ケース

//本書より参照。
//HttpFileCollection(http://goo.gl/yVXB1), 
//HttpPostedFile(http://goo.gl/ZrQsw) は.NET標準クラス。sealedクラス且つpublicなコンストラクタが存在しない。
public class TestTarget
{
  public IList getKSRStreams(HttpFileCollection files){
    ArrayList list = new ArrayList();
    foreach(string name in files){
      HttpPostedFile file = files[name];
      if(file.FileName.EndsWith(".ksr") || (file.FileName.EndsWith(".txt") && file.ContentLength > MIN_LEN)) {
        ...
        list.Add(file.InputStream);
      }
    }
    return list;
  }
}

1,HttpFileCollectionについて調査
・クラス階層: NameObjectCollectionBase抽象クラスを継承している。
・filesの処理: foreachループに使用。
2,HttpPostedFileについて調査
・クラス階層: 自分がルートなsealedなクラス。
・fileの処理: FileName、ContentLength、InputStreamプロパティを呼び出して値取得を行う。

//テストが書けるように対応後

public class OurHttpFileCollection : NameObjectCollectionBase
{
  ...
}

public interface IHttpPostedFile
{
  string FileName();
  int ContentLength();
  Stream InputStream();
}

public class HttpPostedFileWrapper : IHttpPostedFile
{
  private HttpPostedFile source;
  public HttpPostedFileWrapper(HttpPostedFile source){ this.source = source; }
  public string FileName(){ get{return source.FileName;} }
  public int ContentLength(){ get{return source.ContentLength;} }
  public Stream InputStream(){ get{return source.InputStream;} }
}

public class FakeHttpPostedFile : IHttpPostedFile
{
  public FakeHttpPostedFile(string fileName, string contentLength, Stream inputStream){
    this.FileName = filename;
    this.InputStream= inputStream;
    this.contentLength = contentLength;
  }
  public string FileName(){ private set; get; }
  public int ContentLength(){ private set; get; }
  public Stream InputStream(){ private set; get; }
}

public class TestTarget
{
  public IList getKSRStreams(OurHttpFileCollection files){
    ArrayList list = new ArrayList();
    foreach(string name in files){
      IHttpPostedFile file = files[name];
      if(file.FileName.EndsWith(".ksr") || (file.FileName.EndsWith(".txt") && file.ContentLength > MIN_LEN)) {
        ...
        list.Add(file.InputStream);
      }
    }
    return list;
  }
}

*1:そのメソッドがクラスの状態に関わる場合は、publicにしてはいけない。

レガシ―コード改善ガイド4

このクラスをテストハーネスに入れることができません

いらただしいパラメータ

本番コードでは、それ以外に手段がない場合を除き、パラメーターにnullを渡すのはやめる。
nullを渡せるような実装だと、処理の至る所でパラメーターチェック処理が行われているはず。これはまずい。(可読性の問題)
代わりにNullオブジェクトパターンの適用を検討すべし。テストケースではnullを渡してもOK。

いらだたしいグローバルな依存関係

Singletonなクラスを内部で使用しているクラスに対するテスト。
→"Singletonなクラスの制約を緩和する" という発想があまり理解できない。
テスト用の処置ならわかるが、本番でもグローバル変数としてSingletonクラスを使用するのもOK!と書かれているように思える。
グローバルな変数を使用したい→Singletonクラスを作成!という判断に自分がどうしてもならないからか?

紹介されていることをソースに落とすとこんな感じでしょうか?

//シングルトンクラスのインターフェイス
public interface ISingleton
{
  int GetHogegeInt();
  ...
}

//シングルトンクラス
public class HogeSingleton : ISingleton
{
  private static ISingleton instance = null;

  protected HogeSingleton(){}

  //言語によっては、アクセス制限をかけることができる。("テストクラスのアセンブリからのみアクセス可"みたいな感じ)
  public static void SetTestingInstance(ISingleton newInstance)
  {
    instance = newInstance;
  }

  public static ISingleton getInstance()
  {
    if(instance == null){
      instance = new HogeSingleton();
    return instance;
  }

  public int GetHogegeInt()
  {
    ...
  }

  ....

}

//テスト用のシングルトンクラス
public SingletonForTest : ISingleton
{
    public SingletonForTest()
    {
      ...
    }

    public int GetHogegeInt()
    {
      ...
    }

    ...
}

//内部でシングルトンクラスオブジェクトを使用しているクラス
public class UseSingletonProc
{
    public UseSingletonProc()
    {
      int hogeInt = HogeSingleton.GetInstance().GetHogegeInt();
      ...
    }

    public bool ExistValue()
    {
      ...
    }

    ....
}

//テストクラス
public class ATestClass
{
  ...

  //テストケース
  public void TestCase()
  {
    HogeSingleton.SetTestingInstance(new SingletonForTest());
    
    var obj = new UseSingletonProc();
    Assert.AreEqure(true, obj.ExistValue());

    ...
  }

  ...
}


*追記
「いらだたしいグローバルな依存関係」の対応例について、自分なら下のような対応を行う。
「staticなクラスが何かのインターフェイスを実装する」というのを見たことが無いので、落ち着かない。

interface IHogege{ ... }
class Hogege : IHogege {...}
class TestingHogege : IHogege{...}

class HogegeFactory
{
   private static HogegeFactory instance;
   public static HogegeFactory CreateInstance(){ ...}
   public void SetHogegeType(Type hogegeType){...}
   public IHogege Create(...){}
}

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

いつまで経っても変更作業が終わりません

遅延時間

外部ライブラリなどの依存性がひどいシステムのコードは、ビルドする度に未変更箇所すべてもビルドしなおす必要がある。
その間ぼーっと待つことになる。(遅延時間) 改善済コードだとビルド~テスト実行の時間が短いので、即座にフィードバックが得られる。

ビルドの依存関係の排除

あるクラスをテストコード上でインスタンス生成できても、まだ依存関係は存在する。
それはそのクラスオブジェクトを使用するすべてのクラスである。
この依存関係を排除するためには、クラスのインターフェイスを分離する必要がある。
→ publicな操作をInterfaceに移動し、クラスはそれを実装する。
このときにインターフェイスと実装クラスは別プロジェクトに分けること!それをしないとこの変更の意味は半減してしまう!

依存関係逆転の法則(Dependency Inversion Principle : DIP)

インターフェイスに依存する場合、その依存は通常、とても小さく目立たないものです。
インターフェイスを変更しなければ、コードを変更する必要はありません。そして、インターフェイスを変更することは、背後にある実装コードの変更に比べると、はるかに稀です。
インターフェイスがあれば、そのインターフェイスお使用するコードに影響を及ぼすことなく、インターフェイスの実装クラスを編集したり、新しい実装クラスを追加したりすることができます。
このような理由により、具象クラスに依存するよりも、インターフェイスや抽象クラスに依存するほうが優れています。
 変更の少ないものに依存することで、ある変更が巨大なコンパイルを引き起こしてしまう可能性を小さくすることができます。

どうやって機能を追加すればよいのでしょうか?

テスト駆動開発

1,失敗するテストケースを記述する。*1
2,コンパイルする。
3,ビルドエラーになるので、テスト対象のメソッドを追加する。戻り値(がある場合)は適当なものを返すようにする。
4,テスト実行する。失敗することを確認。
5,テスト対象のメソッド実装する。
6,テスト実行する。成功することを確認。
7,重複を確認。必要であればリファクタリングを行う。テスト実行を行い、成功することを確認する。*2
を繰り返す。

差分プログラミング

機能追加を行う度にサブクラスを作成すると、追加した複数の機能に対応させたい要件が発生した場合に困ったことになる。その場合の対応。
最も安直な改善方法の一例として、コンストラクタにパラメーターを渡し、有効機能の切り替えを行うというものが考えられるが、パラメーターが大量に増えた場合に複雑なコードと化するであろう。
これに対応するには、切り替えが必要な操作を別クラスに分離するという方法が対応策の1つとして考えられる。

リスコフの置換原則(LSP)

あるクラスのサブクラスはベースクラスと同様に扱うことができること。利用者側からしたら親子の関係なく使用する。
この原則を違反しないように気をつけることとしては、
1,親クラス(具象クラス)の具象メソッドをオーバーライドしない。
2,オーバーライドする必要がある場合は、親クラスメソッドが呼び出せるようにする。
原則に従っているクラス構成を「正規化された階層」と呼ぶ。

*1:1つのメソッドに対するテスト項目が複数ある場合、それぞれ別のテストケースとして定義すること。

*2:手順5では成功するように、とりあえず実装している。これをこのステップで整備する。

レガシーコード改善ガイド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();
    }
}

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

レガシーコードの定義

"テストの無いコード"すべてをレガシーコードと呼ぶ。

フィードバックを得ながらの作業

システム変更の方法は大きく二種類に分けることができる。
1,編集して祈る → レガシーコード
2,保護して編集する → テストで保護されたコード

単体テストとは

1,単体テストは早く走る必要がある。そうでないと繰り返しテストができない。
(1テストに0.1秒は時間がかかりすぎ。1クラス10件のテストとして、それが3000クラスあると、50分かかる。。。)

2,振る舞いが正しいことを素早く確認するのが単体テストの目的なので、
下記(引用)処理がテストコードに含まれていると目的を達成できない恐れあり。

>他の種類のテストが単体テストの仮面を被っていることもよくある。次に当てはまるものは単体テストではない。
>1,データベースとやり取りする
>2,ネットワークを介した通信をする
>3,ファイルシステムにアクセスする
>4,実行するために特別な環境設定を必要とする(環境設定ファイルの編集など)

※これらのテストが不要なことはなく、むしろ必要である。しかしコード変更毎に走らせるテストコードに含めるのではなく
別管理すべきである。

レガシーコードをテストコード作成できるようにするには?

リファクタリングを行い、データベースやファイルシステムなどへの依存性を取り除く。
テストできるようにするにあたり、不本意な構成に変更する必要性に迫られる場合がある。しかしそれは仕方が無いこととして受け入れるしかない。(今までよりは良くなっているだろう。)

レガシーコードの変更手順

1,変更点を洗い出す。
→ 仕変・機能追加による、振る舞いの変更を実現するために必要な対応と対象箇所一覧を調査する。
2,テストを書く場所を見つける。
3,依存関係を排除する。
→ テストすべき対象(メソッド)を決定する。テストクラスで対象クラスインスタンスを生成できるようにリファクタリングする。
4,テストを書く。
→ 振る舞いを満たしていることを確認するテストケースを生成し、パスすることを確認する。

検出と分離

依存関係の排除に役立つ手法の紹介

擬装オブジェクト(FakeObject)

Strategyパターンを適用して組み込む、テスト用のクラス。

モックオブジェクト(MockObject)

振る舞いを外から設定することができるテスト用のユーティリティクラス。
大抵の言語には対応するモックフレームワークが存在するはず。

接合モデル

接合部:その場所を直接変更しなくても、プログラムの振るまいを変えることの出来る場所である。
種類としては、プリプロセッサ接合部、オブジェクト接合部がある。
許容点:接合部でどの振る舞いを行うか決定するところ。

下記のソースを例に上げると、Hoge.InitializeメソッドでFooLib.CallFooFunction呼び出しだけを行うことをやめたい。

  public class Hoge
  {
    private FooLib foo;  //外部ライブラリ

    public Hoge()
    {
    }

    public void Initialize()
    {
      ....
      foo.CallFooFunction();  //外部ライブラリよびだし。DBアクセスなどの依存関係を含むため、このままではテストコードが作成できない。
      ...
    }
  }

この場合の対処例としては以下のようになる。

  public Interface IFoo
  {
    void CallFooFunction();
  }

  public NothingFoo : IFoo
  {
    public void CallFooFunction(){;}
  }

  public WrappedFoo : IFoo
  {
    private FooLib foo;

    public WrappedFoo(FooLib foo)
    {
      this.foo = foo;
    }

    public void CallFooFunction()
    {
      foo.CallFooFunction();
    }
  }

  public class Hoge
  {
    private IFoo foo = new NothingFoo();

    public Hoge()
    {
    }

    public Hoge(IFoo foo)
    {
      this.foo = foo;
    }

    public void Initialize()
    {
      ....
      foo.CallFooFunction();
      ...
    }
  }

この場合の接合部は、foo.CallFooFunction()となる。(オブジェクト接合部)
外部から処理を取り替えれるように対応することで依存性を取り除いた。

Visual Studio 2010でアプリケーションのパフォーマンス・チューニング

DBアプリケーションのパフォーマンス・チューニング − @ITの内容メモ

クエリのデータ転送量を減らす

不要な項目をSELECT区に含めない。安易に*を使用しない。
SQLServerからの転送量がかなり節約できる。

クエリの内容をデバッグ出力

Entity Frameworkでは、ObjectQueryクラスのToTraceStringメソッドを使用する。

var q = from a in db.Tweets select a;
var trace = ((System.Data.Objects.ObjectQuery)q).ToTraceString();

ObjectQuery(T) クラス (System.Data.Objects)

大量のデータをインポートする方法

テストデータを作成して、それをテーブルにInsertする場合、現実的には2方法が考えられる。

    1. ManagementStudioのオブジェクトエクスプローラから対象テーブルを右クリックし、「上位200行の編集」を選択。編集可能状態でテーブルデータが表示されるので追加データを貼り付ける。
    2. 「データのインポートおよびエクスポート 」ウィザードを使用してデータをインポートする。

普段は手軽なので1方法でインポートしているが、速度が遅いので大量データをインポートする必要がある場合は、2方法を利用すべき。

こんまり式の片付け

テレビで見て感心したので頭に残っていることを書き出しておく。

種類別に片付け

場所ごとに片付けするのではなく、種類ごと(服,本,書類,,,)に片付ける。
整理対象種類のものを一箇所にまとめて片付けする。こうすることでものの量が把握できる。(その多さにびっくりして片付けないと!となる?)

片付けの順番

絶対厳守

    1. 衣類
    2. 書類
    3. 小物
    4. 思い出の品(写真,記念品,,,)

衣類の整理

「部屋着にします」は禁句。
不要なぬいぐるみは、目隠しして捨てる。供養する気持ちで送り出すこと。

小物

不要なコード類(テレビやビデオの線)は原則捨てる。
説明できないものは捨てる。

思い出の品

捨てられないからと言って、実家送りは厳禁。実家を思い出の墓場にしては行けない。
不用品はどこに行っても不要品。



人生がときめく片づけの魔法

人生がときめく片づけの魔法