リファクタリングは“テストコードありき”で考えよう
誤解されたリファクタリング
こんにちは業務共通開発部の田中（翔）です。
最近、職場で「リファクタリング」という言葉をよく聞きます。過去の技術的負債を解消したり、コードを整理してわかりやすくすることは、ソフトウェアの寿命を延ばすうえで重要です。そうした意識がエンジニアの間で広がるのは、良いことだと思います。
しかし一方で、テストコードなしで行われるリファクタリングを目にすることがあります。例えば、テストコードもエビデンスもないまま、単なる手動テストだけでリファクタリング後の振る舞いを確認した、というものです。こうした対応は、単なる「コードの変更」であって、リファクタリングとは言えません。単なる変更で予期せず振る舞いが変われば、単なる無責任なコード改修となり、システムの振る舞いが変わってしまい、バグや障害につながってしまいます。
リファクタリングとは
『リファクタリング 既存のコードを安全に改善する(第2版)』には、次の定義があります。
リファクタリングとは、ソフトウェアの外部の振る舞いを保ったままで、内部の構造を改善していく作業[1]
つまり、単なるコードの整理だけでなく、「元の振る舞いを保証した上での改善」という大前提が必要です。元の振る舞いが保証できないのであれば、単なる変更、あるいは不具合を生み出す「無責任なコード改修」に過ぎません。
そうした「リファクタリングもどき」によってソフトウェアの挙動が変われば、障害が発生し、結果としてプロダクトや組織へ大きなダメージを与えかねません。
リファクタリングのメリットは
『Tidy First?[2]』には、以下のようなリファクタリングするメリットが記載されています。
- 理解力が向上する
- 振る舞いの変更を容易にする
- 変更コストを削減する
簡略化され理解できるコードになることによって、コードの誤解が減ったり、既存のバグを見つけられたりという恩恵を受けることができます。システムは改修を行うにつれて、徐々にスパゲッティ構造のコードが生まれたりして技術的負債を積み上げ、劣化していきます。リファクタリングはこの負債を解消し、システムの劣化の防止にもつながります。
リファクタリングのやり方
リファクタリングの正しい進め方はシンプルです。
- テストコードを先に用意し、現行の振る舞いを保証する
- テストが通る状態でコードを改善する
- 改善後もテストが通れば、振る舞いが保証される
テストコードがない状態でリファクタリングを行う際は、次の手順で進めることを推奨します。
- 現行ロジックの振る舞いを保証できるテストコードを追加する
- リファクタリング後、テストコードが全てパスすることを確認する
例：現行ロジックのテストコードを先に作成する
例えば、以下のATM手数料の計算ロジックを例にします。
ご参考までに作成したコードの言語やツールは以下になります。
- 使用言語：Java17
- テストフレームワーク：Spock2.3
public Fee chargeFee(UseDateTime useDateTime) {
if(this.atmType.equals(AtmType.銀行ATM)){
if(WorkingTime.銀行ATM.isWorkingTime(useDateTime.getUseTime())){
return new Fee(0);
}
return new Fee(110);
}else if(this.atmType.equals(AtmType.コンビニATM)){
if(useDateTime.isSaturdayOrSunday()){
return new Fee(330);
}
if(useDateTime.isWeekDay()){
if(WorkingTime.コンビニATM.isWorkingTime(useDateTime.getUseTime())){
return new Fee(220);
}
return new Fee(330);
}
throw new RuntimeException("コンビニATMの手数料が算出できません");
}
throw new RuntimeException("ATMの種類が特定できません");
}
このコードをリファクタリングしたい場合は、先にテストコードを追加します。次の5パターンのテストを書きました。
def "chargeFee_#usecase"() {
setup:
AtmEntity atmEntity = atmFactory.createAtmEntity(atmType);
UseDateTime useDateTime = new UseDateTime(dateTime)
when:
Fee fee = atmEntity.chargeFee(useDateTime)
then:
fee == expected
where:
usecase | atmType | dateTime || expected
"銀行ATM営業時間内"|AtmType.銀行ATM | LocalDateTime.of(2023, 1, 6, 9, 0, 0) || new Fee(0)
"銀行ATM営業時間外"|AtmType.銀行ATM | LocalDateTime.of(2023, 1, 6, 22, 0, 0) || new Fee(110)
"コンビニATM土日" | AtmType.コンビニATM | LocalDateTime.of(2023,1,7,22,0,0)|| new Fee(330)
"コンビニATM平日_営業時間内" | AtmType.コンビニATM | LocalDateTime.of(2023,1,6,9,0,0)|| new Fee(220)
"コンビニATM平日_営業時間外" | AtmType.コンビニATM | LocalDateTime.of(2023,1,6,21,0,0)|| new Fee(330)
}
上記では各手数料の金額単位のテストを行いました。テストケースの粒度は境界値単位などもあります。品質保証したい単位を考慮してテストケースを作りましょう。
このように、先にテストコードを追加しておくこと で、元の振る舞いが保証されます。そのうえでリファクタリングを行えば、安心して改善できます。
リファクタの実例
今回の例では、銀行ATMとコンビニATMの手数料計算が if 文で複雑に分岐していました。そこで、interface を導入し、ATMごとにクラスを分割することで、条件分岐を減らし、仕様をドメイン単位に整理しました。
リファクタ後のクラス構成は以下になります。
AtmEntityにあったchargeFeeのメソッドを銀行ATMとコンビニATMで分割した実装は以下になります。
AtmEntity
public interface AtmEntity {
Fee chargeFee(UseDateTime useDateTime);
}
銀行ATMEntity
public class 銀行ATMEntity implements AtmEntity{
private final AtmType atmType = AtmType.銀行ATM;
public Fee chargeFee(UseDateTime useDateTime) {
if(WorkingTime.銀行ATM.isWorkingTime(useDateTime.getUseTime())){
return new Fee(0);
}
return new Fee(110);
}
}
コンビニATMEntity
public class コンビニATMEntity implements AtmEntity {
private final AtmType atmType = AtmType.コンビニATM;
public Fee chargeFee(UseDateTime useDateTime) {
if(useDateTime.isSaturdayOrSunday()){
return new Fee(330);
}
if(useDateTime.isWeekDay()){
if(WorkingTime.コンビニATM.isWorkingTime(useDateTime.getUseTime())){
return new Fee(220);
}
return new Fee(330);
}
throw new RuntimeException("コンビニATMの手数料が算出できません");
}
}
このリファクタにより、
- if 文の分岐数を減らし、ネストも浅くできた
- ATMの種類ごとに手数料計算ロジックをクラスへ集約できた
結果として、コードがシンプルになり、可読性・保守性が向上しました。
テストで保証された安心感
実際に行ったPull Requestでは、テストコードを一切変更していません。（個人で作ったため、個人のGitHubのリポジトリを使っており、社内の開発環境と異なります）
リファクタ後も、既存の手数料計算テストがすべて通過しており、元の振る舞いから変わっていないことが確認できました。これは、事前にテストコードを用意していたおかげです。テストを先に書いていたからこそ、構成変更を伴うリファクタでも「業務ロジックの結果は変わらない」と安心して進められました。もしテスト漏れが見つかった場合は、別のマージリクエストで追加すれば十分です。大事なのは、リファクタリング時にプロダクトコードとテストコードを同時に変えないことです。
おわりに
リファクタリングの本質は、元の振る舞いが変わらないことを保証したうえで改善することです。
テストコードがあるからこそ、安心してクラス内のロジックを整理したり、クラス内の構成を変えたりできます。逆にテストがない状態でのリファクタリングは、ただのリスクの高い変更です。
「リファクタリングはテストコードが前提」
これを意識して、変更に強いプロダクトの開発に取り組んでいってもらえたらいいなと思います。
掲載したソースコードはサンプルとになります。本ソースコードを使用することにより発生するいかなる損害や不利益について、当社は一切の責任を負いませんので、自己の責任においてご利用下さい。
Discussion