みなさまこんにちは、社内でアプリ開発を行っているおさかなです。
機能の追加や仕様変更を行っているとコードが煩雑になっていくことがよくあります。また、仕様変更を行う際に当初想定している構成とは異なってくることもあります。これを見て見ぬふりをして開発を進めると、著しくコードの可読性が下がり、バグが起きたときに特定することも難しくなります。今回はそのような事態が起きるのを避け、円滑に開発が進められるように、すぐに実践できるリファクタリングの基本について紹介したいと思います。 リファクタリングとは「既存のコードに対して、外から見た挙動を変更せずプログラムの構造のみを改善すること」を意味します。そのため、変更前と変更後で出力結果が変わるような変更はリファクタリングとは呼びません。
ここで述べている「改善」は非常に広い意味を持ちますが、例えば可読性の低い変数名を変更する、というような些細な修正からプログラムのクラス構成自体を変更する大きなものまで含んでいます。 一般的にリファクタリングでは以下のメリットがあると言われています。 コードが整理され、自分以外の開発者もコードが読みやすくなります コードをシンプルにすることでバグが起きにくくなり、起きたときでも原因の特定がやりやすくなります コードの見通しが良くなるのでテストの作成がやりやすくなったり、テスト漏れが少なくなったりします コード全体を整理することで、重複していたコードがなくなり、同じ機能を使用したい場合に使い回すことができるようになります それではリファクタリングの入門として、リファクタリングを始めるにあたって意識すべきポイントをいくつかご紹介したいと思います。具体的なコード例はC#をベースに記述しています。 コードを書いていると、よくifなどの条件式を使用すると思います。条件式やループは便利なので、ついつい条件の中に条件、さらにその中に条件……という形で追加してしまうことがよくあります。下記のコードは条件分岐がネストしているコード例になります。 複雑なコードでは条件をネストさせることは仕方がないことですが、ネストさせすぎると可読性が下がってしまいます。そこで、「早期にreturn/breakを使用してネストが深くならないようにする」ことを意識しましょう。条件式を工夫することでネストが浅くなり、可読性が向上します。 コードを書いていると、意味の通じない命名、到達することのないコードが見られることがあります。その際はすぐに意味の通じる命名に修正し、到達することがないコードはノイズとなるので削除しましょう。 意味の通じる命名に修正する際も、伝わりやすさを考慮して極力短縮した用語は使用しないようにしましょう。なお、今後使用されることを想定して、現在は必要のないコードを残しておくこともありますが、そうしたコードも極力削除することをおすすめします。いろいろな考え方がありますが、将来的には仕様変更でそのコードが使えないものになっている場合が多いためです。 nullは便利でうまく使いこなすと非常に強力な機能ですが、扱いが難しく不具合の原因となることが多々あります。その複雑さからRustなどの比較的新しい言語では言語自体がそもそもnullをサポートしていない場合や、C++やJavaでも設定でnull安全にすることも可能になっています。
そのため、バグの混入を避けるためにもメソッドでnullを返すものやnullを渡すコードは書かないことを推奨します。 staticメソッドは他のクラスからも簡単にアクセスしてメソッドを使用することができるので非常に便利な機能です。しかし、実はクラス間に不要な依存関係を生んでしまうのでコードの複雑性を高める原因になりがちです。
一般的にソフトウェアアーキテクチャーでは疎結合・高凝集（プログラムの構成として、必要な情報が適切な場所に配置されている度合いを意味する言葉）な構造であることが望ましいとされています。そのため、staticメソッドを使用する際は十分に検討してから使用しましょう。 また、実際にstatic宣言していないメソッドでも、staticをつけて動く場合はstaticメソッドと同様で凝集性を下げています。このようなメソッドを作成しないように意識することで、データとロジックが整理されて可読性の向上につながります。 プログラミング言語では配列やリスト、辞書などのようにデータの集合（コレクション）を扱うことができます。コレクションに対してデータの追加・削除などの処理を行うことが可能ですが、この処理を様々なメソッド内で呼び出していると重複したコードを書く原因になります。 良くない例として、コレクションに対する処理が重複するコードを下記に紹介します。試験の主科目を保持するクラスとしてMainSubjectsManager があり、その中ではメンバーを追加するaddSubject()メソッドが定義されています。もし、今後サブ科目と主科目すべてを保持する際に使用するクラスAllSubjectsManagerを定義することになった場合、AllSubjectsManager クラスにもメンバーを追加するaddSubject()メソッドが必要になることが予想されます。すると、そのクラスにもaddMemberメソッドを定義する必要があり、コードの重複が発生してしまいます。 そこで、コードの重複を避けるためにコレクションを変数として持ち、それに対して処理を行うコレクションクラスの実装を行います。 コレクション側で追加・削除などのメソッドを持たせることで、MainSubjectsManager や AllSubjectsManager はコレクションクラスのメソッドを呼び出すだけで処理を行うことができるようになります。 プログラミング言語における継承は、基底クラスの構造を引き継いでメンバ変数やメソッド作成できるため、うまく使用すると非常に便利な機能です。一方で、派生クラスは基底クラスの構造に大きく依存するため、密結合が生じて壊れやすくなっているとも言うことができます。 そこで、継承の例として以下のコードを紹介します。基底クラスとしてSubjectProperty クラスを作成し、それを継承してMathSubjectProperty クラスを作成しています。下記コードは、動作は問題ありませんが、今後SubjectProperty クラスにメンバ変数やメソッドを追加する必要が生じたと時に、MathSubjectProperty クラスも変更する必要があり修正規模が大きくなってしまいます。 そこで、クラス間の密結合を避けるために、継承の代わりに委譲を使用する方法が推奨されています。委譲とは、継承による基底クラスと派生クラスの依存関係を作らず、メンバ変数としてクラスを持たせるデータ構造です。先ほどの継承を委譲で書き換えると下記のようになります。 委譲にすることで、もしSubjectProperty クラスに変更が生じた場合でもMathSubjectProperty に直接修正が及ぶことはありません。 ここまで、リファクタリングで意識することを紹介しましたが、実際にどのタイミングでリファクタリングを行うべきでしょうか。理想を言うと、日常的にリファクタリングを行うことが望ましいですが、開発と並行して実施するのは現実的とは言えません。
そこで、本稿では下記の5つのタイミングでリファクタリングを実施することを推奨します。 既存のコードに対して機能追加する場合に、柔軟な設計になっておらず機能の追加が難しい場合があります。そのときはまずデータ構造の見直しを行い、その後に機能の追加を進めていきます。なお、リファクタリングはあくまでも挙動は変更しないので、機能追加とリファクタリングは別々に行いましょう。 コードを書いているときに問題を見つけたときも絶好のリファクタリングの機会です。工数にもよりますが、簡単なモノであれば問題は後回しにせずに対応するのがおすすめです。 チーム開発を行っている場合はコードレビューのタイミングでもリファクタリングをすることを推奨します。良くない構造のプログラムが開発ブランチにマージされてしまうと、バグの原因にもつながるのでリファクタリングの視点でレビューすることが大事です。 リファクタリングのタイミングとしてメソッドや引数に注目することも大事です。一般的にメソッドの引数が5個以上になっているとクラスが肥大化し始めていると言われます。
また、メソッドの引数が多いと、引数の場所を間違える原因にもつながるため多くても4個程度に抑えるのが理想です。 privateなメソッドが多いということは、クラスがかなり大きくなっているという危険信号でもあります。一般的に一つのクラスは一つの責任しか負わないようにする（単一責任の原則）という考え方がプログラミングでは良いとされています。
privateメソッドは5~7個以上になっているとクラスが肥大化し始めている目安だと言われます。privateなメソッドが増えているクラスは改めてその役割を再検討し、クラス分割などを行いましょう。 本稿ではリファクタリングの考え方と基本的な実践方法について紹介しました。コードを書いて機能を実装することも大切ですが、日常的にコードの見直しを行ってバグの起こりにくいコードに変えていくことも同じくらい大切です。
リファクタリングは奥が深く人によって様々な考え方があるので、自分の手で一度調べてみると視野が広がって面白いのではないでしょうか。
それではまた次回の記事でお会いしましょう！
リファクタリングとは
今日から始められるリファクタイング入門
1. 条件分岐によるネストを回避する
int totalScore = 0;
int mainSubjectScore = 0;
foreach(var subject in subjects){
if(subject.isTaken){
if(subject.isMain){
totalScore += subject.score;
mainSubjectScore += subject.score;
break;
} else{
totalScore += subject.score;
break;
}
}
else{
break;
}
}
int totalScore = 0;
int mainSubjectScore = 0;
foreach(var subject in subjects){
if(!subject.isTaken) return;
if(subject.isMain){
mainSubjectScore += subject.score;
continue;
}
totalScore += subject.score;
}
2. 不要なコード・意味の通じない命名の排除
Grade grade;
void testResult(int score){
if(score > 500){
score = 500;
}
if(mainSubjectScore < 300 ){
grade = Grade.B;
}
else if(mainSubjectScore < 400){
grade = Grade.A;
}
else if(mainSubjectScore == 500){
grade = Grade.S;
}
// 入ることの無い条件式
else if(mainSubjectScore > 500){
grade = Grade.S;
}
}
int calculateAverageMainSubjectScore(int totalScore){
// 意味の通じない変数名
int tmp = 0;
tmp = totalScore / 5;
return tmp;
}
3. nullを返さない、渡さない
int getSubjectAverageScore(Subject subject){
// nullが入る可能性がある場合はnullチェックが必要
if(subject != null){
return subject.averageScore;
}
return 0;
}
4. staticメソッドは安易に使用しない
class Subject{
private int average;
...
int addScore(int score1, int score2){
return score1 + score2;
}
}
class Subject{
private int average;
...
// staticを付けても問題なく動作する
static int addScore(int score1, int score2){
return score1 + score2;
}
}
5. コレクション処理では重複コードを避ける
class MainSubjectsManager{
void addSubject(List<Subject> subjects, Subject subject){
// すでに存在していたら追加しない
if(subjects.Contains(subject)){
return;
}
subjects.Add(subject);
}
}
class AllSubjectsManager{
void addSubject(List<Subject> subjects, Subject subject){
// すでに存在していたら追加しない
if(subjects.Contains(subject)){
return;
}
subjects.Add(subject);
}
}
class Subjects{
private List<Subject> subjects;
...
void addSubject(Subject subject){
// すでに存在していたら追加しない
if(subjects.Contains(subject)){
return;
}
subjects.Add(subject);
}
}
class MainSubjectsManager{
private Subjects subjects;
...
void addSubject(Subject subject){
subjects.addSubject(subject);
}
}
class AllSubjectsManager{
private Subjects subjects;
...
void addSubject(Subject subject){
subjects.addSubject(subject);
}
}
6. 継承より委譲を使用する
class SubjectProperty{
string Name { get; set; }
int averageScore { get; set; }
// 再試験になる点数の算出
int calculateRetestScore(){...}
}
class MathSubjectProperty : SubjectProperty{
// 数学では再試験の基準を緩くする
int calculateRetestScore(){
return super.calculateRetestScore() - 10;
}
}
class MathSubjectProperty{
private SubjectProperty subject;
int calculateRetestScore(){
return subject.calculateRetestScore() - 10;
}
}
リファクタリングのタイミング
既存コードに機能を追加するとき
プログラム中に問題を見つけたとき
コードレビューのとき
メソッドの引数が多いとき
privateなメソッドが多いとき
まとめ
