見出し画像

テストコードを減らすためのリファクタリングテクニック

ようやく本題です。
今日はこの話をしたいがためにサンプルコードの記事を書いていたのですが、色々やっていくうちに気付きがあったので、別記事で色々と書き足す事にしました。

# 謝辞

書き終えて気付いたんですが、言ってる内容がほとんどこれでした。

# サンプルコード

このうち、

## リファクタリング前

## リファクタリング後

を取り上げます。
どちらも、不正な値が入力された時以外は同じ動作をするコードです。

# リファクタリングとは?

概要はWikiにぶん投げます。

私の解釈で恐縮ですが、ざっくり解説すると仕様変更(大体の場合、要件定義で発見されなかった隠れ仕様や、開発期間中の法改正などにより変更を余儀なくされたもの)に振り回される事が多いシステム開発において、最新の状態で最適化することを指します。
が、ここで取り上げるのは、テストコードを減らすためのリファクタリングです。

# なぜテストをするのか?

これも概要は外部にお任せします。

※本記事は2019年8月18日時点で作成されたものなので、プログラミングに関係ない記事でもセーフなのか運営のお目溢しの可能性がありますので、将来的にリンク切れになる可能性があります。

こういった問題を解消するためにテストを行うのですが、品質評価を行う側ではなく、開発者として単純に「無意味なテスト項目を減らすため」と解釈します。
(この記事では、品質評価を実施する上で、この考え方の善悪について議論はしません)

# 本論:開発者のテスト(単体テスト)

単純にkeydownという関数をテストしたい場合を考えます。
keydown関数は、受け取ったキーが何かを表示させる関数です。
keydownで対応するキーを押すと、対応する文字列を返します。
たとえば、カーソルキーの↑キーを押したら、画面に↑を出力できることが期待値です。
今回は、カーソルキー「↑」「↓」「←」「→」を対応する事になりました。

また、不正な値を入力された時は気にしない事にします。
(※実際の開発現場において、仕様書に書いてない場合があった時はシステム設計者に問い合わせましょう)

## リファクタリング前

if文(条件分岐)が5通りあるので、単体テストも最低6ケース作る必要があります。
雑にまとめると、

- 入力がされ、何かを出力されること(機能テスト)
- ↑キーを押すと、↑が出力されること(仕様テスト)
- ↓キーを押すと、↓が出力されること(仕様テスト)
- ←キーを押すと、←が出力されること(仕様テスト)
- →キーを押すと、→が出力されること(仕様テスト)
- それ以外のキーを押す(異常系テスト)

となります。

ここでいう機能テストとは最低限の動作を確認することで、仕様テストは要求された仕様を満たすためのテストです。
異常系テストとは、ざっくり正常系ではない場合のテストです。ここでは入力の値が仕様書にないものとしました。

## リファクタリング後

関数をテストするだけならもっと簡単にできます。

- 入力がされ、何かを出力できること(機能テスト)
- cursorに該当するキーを押す(仕様テスト)
- cursorに該当しないキーを押す(異常系テスト)

厳密に行うならリファクタリング前と同じテストを行いますが、条件分岐がないので関数の単体テストとしてはこれだけで完了できます。

## リファクタリングで何が嬉しいか(テスト以外)

機能改修を求められた時のメンテナンス性の向上が最も分かりやすいです。
システムの全貌を知らなくても、関数の振る舞いが分かっていれば、そこだけ直せば機能改修が容易になります。
関数の作成者以外にも、使い方が分かる人なら誰でも作り直す事もできます。

例えば、もしプロジェクトで少しずつリファクタリングを進めている場合は、古い関数と新しい関数が同時に存在しているはずなので、古い関数を使っている場所に気付きを与えたり、書き換える前と後の違いをドキュメントに興しておきましょう。

というのも、手が回らなくなった時に、リファクタリングをしていた・していない判別が出来なくなる上、イレギュラーケースが起こった時の運用ルールが分かりにくくなるからです。
場合によっては、リファクタリングをしていた担当者が転属などにより、誰にもメンテナンスされないコードが出てきてしまいかねません。

## リファクタリングで何が嬉しいか(プロジェクト以外)

主に個人を想定していますが、リファクタリングは積極的に行っていくべきです。
他の人が書いたコードや、昔の自分が書いたコードを見直すとおかしいポイントや、勉強になる書き方などが見えてきます。
実際に写経してみて、「これよりもっと良い書き方があるはず」と思って調べてみましょう。
思いもよらなかった方法、もっとイケてる書き方が見つかります。

## 安易にリファクタリングをしてはいけないケース

既にkeydownがどこかで使われていて、出力先のOUTPUT_IDが複数あるケースです。今回は単一のIDを共通して使うので、keydown関数内で決め打ちしていますが、これを引数を取るケースなどが考えられます。
これは「リファクタリングをしてはいけない」というよりは「初期段階から一つの関数に煩雑な処理をさせてはいけない」と言い換える事もできます。
が、もうやってしまったものは仕方がないので、いったんTODOリストかタスクボードで周知し、プロジェクト全体で負の遺産を取り除こうという流れになった時に行うべきです。

今回はJavaScriptなので、keydownの引数を一つ増やせば解決できます。そして、この仕組みを知っている人はよくやりがちです。
ただし、どこかでウッカリkeydown("引数")とやろうものなら、想定した結果が返ってこない事になります。
(本来なら一緒に直すべき点ですが、だいたい気付かれないです)

# コラム:開発者が考えるべきテスト

実行しなくても意味がわかるようにプログラムを見直す事が重要です。
今回は小さな関数なので、リファクタリング前の状態でも混乱しないですが、これが多段(多層)構成になってきたり、単純に行数が増えると見にくくなってきます。

よくあるのが、

- 入力された値がnullなのかundefinedなのか0なのか(どれもif文ではfalseとして扱われるもの)
- 値の正しさ(今回の場合、テンキーの「8」「2」「4」「6」、FPSゲームでいう「W」「S」「A」「D」は同じか)
- データの加工が必要か
  - データの加工はkeydown関数で行うべきではないが、誰かがやらなければならない必要な作業である場合

に、どう対処すべきか悩む事があります。

## 実話

とあるプロダクトの運用保守において、頻繁にエラーを吐く600行近い関数がありました。
ななめ読みをしていると、文字列を整形したり補完したり、出力先を管理したりと結構重い責任を持っていたのです。
一行ずつ手続き型で書いてくれていれば頑張って読む気になりますが、役割を勘違いした子クラスがひしめき、何のためにライブラリを使っているか分からず、もう誰もメンテが出来なくなってしまった巨大クラスをリバースエンジニアリングしてマイクロモジュールに作り直した事があります。
結果、どこにバグがあったのか突き止めることはできましたが、一つの関数を作り直すのに二週間も掛けたという苦い経験があります。

プログラムを書く時は、コメントの必要がないほど分かりやすいものがベストですが、自分が読みやすいコメントや書き方になってしまっている可能性を除外しています。
プロダクトで使うソースコードなら自分が読みやすいと思っても、敢えてコメントを残してくれれば多少は読みやすくなったし、クラスの役割や理由をドキュメントに興してくれれば後任の方が保守する気になります。

読んだことがない人はもう居ないと思いますが、念のためにいつもの貼っときます。

のむらがあなたの役に立つ記事を書くためのコーヒーを一杯奢ってくれませんか? サポートをすると、のむらの記事を使って、あなたの記事を盛り上げてみませんか? また、有料記事などいただいた収益はすべて、あなたの代わりにアプリやツールを買って色々検証をして記事にするために使っています。