レビューで指摘された内容をそのまま放流するシリーズ。(今回は実はレビューじゃないけど)まとめページ もあります。
NUnit のテストで、インスタンスの型が期待通りの型になっているか確認するコードを下記のように書いていました。
private Type _hogeType
[SetUp]
public void Init()
{
_hogeType = typeof(Hoge);
}
public void HogeTest1()
{
var instance = HogeFugaFactory.Create("HOGE");
Assert.IsInstanceOf(_hogeType, instance);
}
// 以降似たようなテストが続く
このコードは一度マージされました。
そして数か月後に発掘され、コメントをもらいました。
このような書き方はテストの見通しを悪くします。想定されるexpectedの値は直接定義するべきです。
コメントを見て、自分もそうだと思いましたw。
正直なぜ Init() の中で一度メンバ変数に落とし込んだのか、ちゃんと覚えていないのですが、推測するに typeof(Hoge)
の処理を何度も書きたくなかったのだと思われます。
typeof(Hoge)
を毎回書かないようにする手段としてメンバ変数に落とし込んだと思われるわけですが、実はこれ、IsInstanceOf の書き方で解決することができることも教えてもらいました。
つまり、こう書けばすっきり書けたわけです。
public void HogeTest1()
{
var instance = HogeFugaFactory.Create("HOGE");
Assert.IsInstanceOf<Hoge>(instance);
}
アサートで比較するものはべた書きのほうが分かりやすいです。変数化すると定義元を確認しないといけないのでテスト内容が分かりにくくなります。
同じような指摘を RSpec でも受けた経験があるので、この話はテスト固有の話でも無いような気がします。
ということで、今後は同じような指摘をもうもらわないようにしたいと思います!(フリじゃないよ!)