冗長なif-elseをリファクタリングする👀
はじめに
リファクタリング技術を勉強していた際に標題の手法がテーマに挙がっていたので、
今回はこれを記事にしてみたいと思います。
なお今回のリファクタリング手法は、
-
タイプコードをクラスに置き換える
-
コードをクラスに移す
-
sealedとswitchを用いた網羅性チェック
こういった手法を組み合わさています。
before 〜 リファクタリング前 〜
先ずは以下のコードを見てください。
/// リファクタリング前
class Employee {
String type;
double monthlySalary;
int hoursWorked;
double hourlyRate;
Employee(this.type, {this.monthlySalary = 0, this.hoursWorked = 0, this.hourlyRate = 0});
}
double calculateSalary(Employee employee) {
if (employee.type == "Manager") {
// マネージャーの給与計算
return employee.monthlySalary;
} else if (employee.type == "Engineer") {
// エンジニアの給与計算
return employee.monthlySalary;
} else if (employee.type == "PartTime") {
// パートタイマーの給与計算
return employee.hoursWorked * employee.hourlyRate;
} else {
throw Exception("Invalid employee type");
}
}
void main() {
final manager = Employee("Manager", monthlySalary: 500000);
final engineer = Employee("Engineer", monthlySalary: 400000);
final partTimer = Employee("PartTime", hoursWorked: 160, hourlyRate: 1500);
print("Manager Salary: ${calculateSalary(manager)}");
print("Engineer Salary: ${calculateSalary(engineer)}");
print("Part-timer Salary: ${calculateSalary(partTimer)}");
}
一見特に問題のないコードであり、このようなif-else文や連続したif文をコーディングすることもあるかとは思います。
しかし、その冗長性ゆえにコード全体の見通しが悪く、今後の可読性やメンテナンス性に問題が生じる懸念があります。
加えて、enumの列挙子が増えたり減ったりした際にはそれに依存するコードにも影響は及びますが、
そのコード(今回はcalculateSalary関数)に手を加えてしまうことで元の振る舞いを保証できなくなったり、それがしづらくなると言った変容容易性の点での懸念、つまりオープンクローズドの法則に反してしまう懸念があります。
更にこの関数の引数や処理内容に着目してみると、全てenumとその列挙子を用いたものであることが分かります。
現在はそれ程ではないかも知れませんが、今後のことを考えると、関心事・役割・責務に基づいてひとまとまりにしていくと言った分離分解の方向性、すなわち単一責任に基づいたクラスへの昇格を考えた方が良さそうです。
また、これは本筋とはズレますが、enumを使うのであればswitchと組み合わせることで網羅性チェックを効かせることも出来ますが、今回のコードではそのようにコーディングしていないのでその強味を発揮できておらず、今後の実装漏れのリスクもありますね。
after 〜 リファクタリング後 〜
先述の懸念点やリスクを鑑みて以下のようにリファクタリングしてみます。
/// リファクタリング後
// 1. タイプコードをクラスに変換する: 共通のインターフェースを定義
abstract interface class EmployeeType {
double calculateSalary();
}
// 役職ごとのクラス
class Manager implements EmployeeType {
double monthlySalary;
Manager(this.monthlySalary);
double calculateSalary() => monthlySalary;
}
class Engineer implements EmployeeType {
double monthlySalary;
Engineer(this.monthlySalary);
double calculateSalary() => monthlySalary;
}
class PartTime implements EmployeeType {
int hoursWorked;
double hourlyRate;
PartTime(this.hoursWorked, this.hourlyRate);
double calculateSalary() => hoursWorked * hourlyRate;
}
class Employee {
// 2. メソッドをクラスに移す: Employeeクラスが給与計算ロジックを持つ
EmployeeType type;
Employee(this.type);
double getSalary() {
return type.calculateSalary();
}
}
void main() {
final manager = Employee(Manager(500000));
final engineer = Employee(Engineer(400000));
final partTimer = Employee(PartTime(160, 1500));
print("Manager Salary: ${manager.getSalary()}");
print("Engineer Salary: ${engineer.getSalary()}");
print("Part-timer Salary: ${partTimer.getSalary()}");
}
先述のenumに該当する部分はabstract interfaceで定義し直しています。
このように定義をすることで各具象クラスに対して実装の強制力を発揮しつつ、自身のインスタンス化を禁止することが出来ます。
次いで、各実装先の具象クラスに行く前にEmployeeクラスを見てみます。
このクラスには先述のインターフェースであるEmployeeTypeがプロパティとして定義されており、
getSalaryメソッドの中で参照されています。
これは依存関係逆転の原則に則ったものであり、
こうすることで、この抽象的なプロパティを介して各々の具象的な振る舞いを表現することができ、可読性やメンテナンス性が高まるだけでなく変更容易性も高めることが出来ます。
加えて先述のリファクタリング前のコードと比較しますと、リファクタリング後は給与取得のロジック、
計算済みの結果を参照するだけで計算そのものは各具象クラスの責務になっていますが、
これをEmployeeクラスの関心事・役割・責務として抽出しています。
リファクタリング前の方はこのような給与取得のロジックは特別に存在はせず、
enum列挙子のプロパティの違いにより表現されていました。
その違いを表現するために各プロパティの初期値が0として定義されていましたが、
こうすると列挙子の増加やプロパティの増加と言ったenum内の変化に対して柔軟に対応しづらくなる時期が来た際に脆さが露呈してしまう懸念もあったので、
関心事・役割・責務に応じてコードをクラスに移行させた(抽象化によりクラスへ昇格させた)リファクタリング後のコードの方が不確実性に対して柔軟に対応出来そうですね。
更に、インターフェースを実装した各具象クラスを見てみますと、これらはリファクタリング前のenumの列挙子に対応してクラス化されていることが分かります。
詳細は僭越ながら下記の記事に譲りますが、
こうすることで各列挙子に固有の振る舞い(今回は各具象クラスごとの給与計算の方法)を持たせることができ、
かつ、リファクタリング前のcalculateSalaryメソッドが孕んでいたenumとその列挙子を使う条件分岐ロジックにおけるオープンクローズドの原則に反するような懸念点についても、
各具象クラスへの昇格と固有の振る舞いの実装により変化に対して強くなっており、何より条件分岐を挟む必要性が無くなっています。
リファクタリング前のコードはif-elseにより列挙子ごとの給与計算を表現していましたが、これでは列挙子の増減に対して脆弱でした。
ifは条件分岐そのものを行いますが、if-elseは分岐を繰り返すことによる決断の先送りを行えるため、それによる冗長性の発生と可読性やメンテナンス性の悪化、変更容易性の悪化を招いてしまう懸念がありました。
リファクタリング後はこれらを解消することが出来ており、冗長性が無くなって可読性やメンテナンス性が高まり、変更容易性も改善されています。
網羅性チェックも効かせたい
先述のリファクタリング後のコードでも可読性やメンテナンス性、変更容易性は高まっていますが、
enum + switchによる網羅性チェック
と言った強味は無くなっています。
リファクタリング前のコードにもこちらの振る舞いはありませんでしたが、そう言ったリファクタリングの可能性は秘めていた訳ではありました。
開発においては網羅性チェックを効かせたロジックを実装するような場面が多いので、
リファクタリング後のコードでも何とか同じ様なことを実現してみたいですね。
そこで以下のように書き換えてみましょう。
/// sealed classを用いた書き方
// 1. タイプコードをクラスに変換する: 共通のインターフェースとなるsealed classを定義
sealed class EmployeeType {
// 抽象メソッドとして定義し、各サブクラスでの実装を強制
// 各サブクラスはこの実装により個々の振る舞いを表現することができる
double calculateSalary();
}
// 2. sealed classを継承するサブクラス（役職ごとのクラス）を定義
// これらのサブクラスは、他のファイルからは継承できない
class Manager extends EmployeeType {
double monthlySalary;
Manager(this.monthlySalary);
double calculateSalary() => monthlySalary;
}
class Engineer extends EmployeeType {
double monthlySalary;
Engineer(this.monthlySalary);
double calculateSalary() => monthlySalary;
}
class PartTime extends EmployeeType {
int hoursWorked;
double hourlyRate;
PartTime(this.hoursWorked, this.hourlyRate);
double calculateSalary() => hoursWorked * hourlyRate;
}
class Employee {
// 3. メソッドをクラスに移す: Employeeクラスが給与計算ロジックを持つ
EmployeeType type;
Employee(this.type);
double getSalary() {
return type.calculateSalary();
}
// switch文を用いた網羅性チェック
// sealed classのおかげで、すべてのサブクラスをカバーしているかコンパイル時にチェックされる
String getEmployeeDescription() {
return switch (type) {
Manager m => "管理職: 月給 ${m.monthlySalary}円",
Engineer e => "エンジニア: 月給 ${e.monthlySalary}円",
PartTime p => "パートタイム: 時給 ${p.hourlyRate}円 × ${p.hoursWorked}時間",
// sealed classなので、すべてのケースを網羅していないとコンパイルエラーになる
};
}
// 別の例: 昇給の可否を判定
bool isEligibleForRaise() {
return switch (type) {
Manager() => true, // 管理職は昇給対象
Engineer() => true, // エンジニアも昇給対象
PartTime() => false, // パートタイムは昇給対象外
};
}
}
void main() {
final manager = Employee(Manager(500000));
final engineer = Employee(Engineer(400000));
final partTimer = Employee(PartTime(160, 1500));
print("Manager Salary: ${manager.getSalary()}");
print("Engineer Salary: ${engineer.getSalary()}");
print("Part-timer Salary: ${partTimer.getSalary()}");
print("\n--- switch文による網羅性チェックのデモ ---");
print(manager.getEmployeeDescription());
print("昇給対象: ${manager.isEligibleForRaise()}");
print(engineer.getEmployeeDescription());
print("昇給対象: ${engineer.isEligibleForRaise()}");
print(partTimer.getEmployeeDescription());
print("昇給対象: ${partTimer.isEligibleForRaise()}");
}
詳細な解説は僭越ながら以下の記事に譲りますが、
abstract interfaceで定義されていたEmployeeTypeをsealedで定義しなおすことで、
固有の振る舞いを持たせたいタイプコードをクラスに変換させつつ、
sealedの特性を活かして網羅性チェックを効かせることができるようになっています。
Employeeクラスの実装意図も効いているので冗長なif-elseの解消にも成功しており、
より表現力が高まっているものと思われます。
if文が連続したり、if-elseが冗長になってきた際はこのようなリファクタリング手法も良いかもしれませんね。
参考
Discussion