LoginSignup
42
35

More than 3 years have passed since last update.

Code Smell とリファクタリング技法について調べてみた

Last updated at Posted at 2020-01-05

Code Smellとは?

以前(2019年8月頃)、CSD (Certified Scrum Developer) 研修を受講しました。非常にためになる研修で、その研修の中でCode Smellというものについて学びました。
Code Smellとは、ソースコードにリファクタリングすべき問題が存在することを示す何らかの兆候のことです。
CSD研修ではペアプロでTDDで開発を行い、開発したコードからCode Smellを嗅ぎ取ったらリファクタリングして取り除く。という開発の流れを教わりました。
研修受講後、このCode Smellについてもっと知りたいと思ってググったりしたのですが、日本語で解説しているサイトが少ないのが現状です。
以下のサイトが、Code Smellとリファクタリングの技法について体系的にまとめていると思いました。

このサイトでは、以下5つの大分類を元に、それに関連するCode Smellを紹介しています。

ここから先は、このページをベースに、各Code Smellを翻訳したものを記載しています。このページだけだとよくわからないこともあったので、その場合は別のページと組み合わせたものを記載しています。
Code Smellを解消するために参考となるリファクタリング技法や、その他参考になりそうな概念もこの記事の最後の方に記載しているので合わせて見てください。

Bloaters

Bloaters(膨れ上がったコード)
作業が困難になってしまうほど巨大になりすぎてしまったコード、メソッド、およびクラスのこと。
通常、これらのCode Smellはすぐには現れず、プログラムに改修を積み重ねること(そして特に誰もそれらを根絶する努力をしないとき)で現れます。

Long Method

Long Method(長すぎるメソッド)

兆候と症状
メソッドの行数が多すぎる状態。一般に、1メソッドの行数が10行を超える場合、短くすることができないかを考える必要があります。

問題の理由
メソッドには常に何かしらの処理が追加され続けますが、処理が削除されるということはめったにありません。コードは読むよりも書く方が簡単なので、この Code Smell は、メソッドが異常なほど大きくなるまで気付かれないままです。
精神的には、既存のメソッドに処理を追加するよりも新しいメソッドを作成する方が難しく感じられます。たった2行の処理を追加するためだけに新しいメソッドを1つ追加するのが難しい場合などです。このようにして既存のメソッドに追加されたコードがスパゲッティコードの始まりになります。

対処
経験則として、メソッド内の何かにコメントする必要があると感じた場合、このコードをメソッド内から抽出して新しいメソッドを作成する必要があります。説明が必要な場合は、1行であっても別の方法に分割することができます。また、メソッドにわかりやすい名前が付けられている場合、コードを調べてその機能を確認する必要はありません。

  • メソッド本体の長さを短くするには、「Extract Method」を使用します。
  • ローカル変数とパラメーターがメソッドの抽出に干渉する場合は、「Replace Temp with Query」、「Introduce Parameter Object」、または「Preserve Whole Object」を使用します。
  • 上記のレシピがどれも役に立たない場合は、「Replace Method with Method Object」を適用してメソッド全体を別のオブジェクトに移動してみてください。

  • 条件演算子(3項演算子)とループは、コードを別のメソッドに移動できる良い手がかりです。3項演算子の場合は、「Decompose Conditional」を使用します。ループが邪魔な場合は、「Extract Method」を試してください。

効果

  • オブジェクト指向プログラミングにおいて、メソッドが短い方がそのクラスの寿命は長くなります。メソッドが長くなるとそれを理解することが難しくなり、メンテナンスも困難になります。
  • さらに、メソッドが長くなるとコードの重複を発見するのが難しくなります。

Performance
リファクタリングによってメソッドが増えることがあります。
多くの人は、メソッドが増えると処理性能に影響があると主張します。しかしほとんどの場合、処理性能に与える影響はごくわずかです。
さらに、明確で理解可能なコードが得られたので、コードが再構築され、必要に応じて実際のパフォーマンスを向上させるための真に効果的な方法を見つける可能性が高くなります。

Large Class

Large Class(巨大なクラス)

兆候と症状
多くのフィールド、メソッド、コード行数を持つクラス。

問題の理由
通常、クラスは最初は小さいものから始まります。しかし、時間の経過と共にプログラムは成長し、肥大化していきます。
Code Smell の一つである Long Method の場合もそうですが、プログラマーは通常、その機能の新しいクラスを作成するよりも、既存のクラスに新しい機能を配置する方が負担が少ないと考えます。

対処
クラスがあまりにも多くの機能を持ってしまっている場合、それを分割することを考えてください。

  • Extract Class」は、大きなクラスの動作の一部を別のコンポーネントに分割できる場合に役立ちます。
  • Extract Subclass」は、大きなクラスの一部を異なる方法で実装できる場合や、まれに使用される場合に役立ちます。
  • Extract Interface」は、利用可能な操作と動作のリストを保持する必要がある場合に役立ちます。
  • 大きなクラスがグラフィカルインターフェイスを担当している場合、そのデータと動作の一部を別のドメインオブジェクトに移動しようとする場合があります。その際、一部のデータのコピーを2か所に保存し、データの一貫性を保つ必要がある場合があります。「Duplicate Observed Data」はこれを行う方法を提供します。

効果

  • これらのクラスのリファクタリングにより、開発者はクラスの多数の属性を覚える必要がなくなります。
  • 多くの場合、大きなクラスをパーツに分割すると、コードと機能の重複を避けられます。

Primitive Obsession

Primitive Obsession(基本データ型への執着)

兆候と症状

  • 単純なタスク(通貨、範囲、電話番号用の特別な文字列など)に小さなクラスを作る代わりにプリミティブ型(intやlongなど)を使用する。
  • コーディング情報への定数の使用(管理者権限を持つユーザーを参照するための定数USER_ADMIN_ROLE = 1など)。
  • データ配列で使用するフィールド名としての文字列定数の使用。

問題の理由
他のほとんどのCode Smell同様、Primitive Obsessionは弱さの瞬間に生まれます。
例えば、クラス内にプリミティブフィールドを作成するような行為です。プリミティブフィールドの作成は、新しいクラスを作成するよりもはるかに簡単です。フィールドが必要になるたびにプリミティブフィールドが追加されます。追加を繰り返した結果、クラスは巨大で扱いにくくなっていきます。

プリミティブは、型に”似せる”ためにもよく使用されます。個別のデータ型の代わりに、いくつかのエンティティの許容値のリストを形成する一連の数値または文字列を定数で宣言します。これらの定数には、わかりやすい名前を付けることができます。これは広く普及してしまっているやり方です。

プリミティブ型を使用した別の質の悪い例は、フィールドシミュレーションです。多様なデータの大規模な配列をクラスに含め、このデータを取得するための配列インデックスとして、クラスで指定した文字列定数が使用されます。

対処
さまざまなプリミティブフィールドがある場合、それらのいくつかを論理的に独自のクラスにグループ化できる場合があります。さらに、このデータに関連付けられた振る舞いもそのクラスに移動するとより良いです。このタスクでは、「Replace Data Value with Object」を試してください。

効果

  • プリミティブの代わりにオブジェクトを使用することで、コードがより柔軟になります。
  • コードが理解しやすくなり、プログラムの構成が改善されます。特定のデータに対する操作が分散せず、同じ場所にあります。定数の意味と、それらが配列内にある意図について推測する必要がなくなります。
  • 重複コードの発見が容易になります。

Long Parameter List

Long Parameter List(長すぎるパラメータリスト)

兆候と症状
メソッドの3つまたは4つ以上のパラメーター。

問題の理由
Long Parameter Listは、いくつかの種類のアルゴリズムが単一のメソッドに統合された後に発生する場合があります。実行されるアルゴリズムとその方法を制御するために、Long Parameter Listが作成されている場合があります。

Long parameter listsは、クラスを互いに独立させる努力の副産物でもあります。たとえば、メソッドに必要な特定のオブジェクトを生成するコードをメソッド内部から呼び出し元に移動した場合、生成されたオブジェクトは引数としてメソッドに渡されます。これにより、元のクラスはオブジェクト間の関係を認識しなくなり、依存関係が減少します。ただし、このようなオブジェクトがいくつも作成された場合、各オブジェクトには独自のパラメーターが必要になります。つまり、パラメーターリストが長くなります。
そのようなリストを理解することは難しく、それらが長くなるにつれて矛盾し、使いにくくなります。Long parameter listsの代わりに、メソッドは自オブジェクトのデータを使用できます。現在のオブジェクトに必要なデータがすべて含まれていない場合は、必要なデータを取得する別オブジェクトをメソッドパラメーターとして渡すことができます。

対処
パラメーターに渡される値を確認します。引数の一部が別のオブジェクトのメソッド呼び出しの結果である場合は、「Replace Parameter with Method Call」を使用します。このリファクタリングによって作成されたオブジェクトは、対象のクラスのフィールドに配置するか、メソッドパラメーターとして渡すことができます。

  • 別のオブジェクトから受け取ったデータのグループをパラメーターとして渡す代わりに、「Preserve Whole Object」を使用して、オブジェクト自体をメソッドに渡します。

  • 関係のないデータ要素が複数ある場合、「Introduce Parameter Object」を介してそれらを単一のパラメータオブジェクトにマージできる場合があります。

効果

  • コードが読みやすく、短くなる。
  • リファクタリングすることでコードの重複に気づくきっかけになる。

Data Clumps

Data Clumps(データの塊)

兆候と症状
同一の変数グループ(データベースに接続するためのパラメーターなど)がコードの異なる部分に散らばっている場合があります。このData Clumpsは、独自のクラスに変換する必要があります。

問題の理由
多くの場合、これらのデータグループは、不十分なプログラム構造またはコピペプログラミングが原因です。
一部のデータがData Clumpsであるかどうかを確認したい場合は、データ値の1つを削除し、他の値が依然として意味をなすかどうかを確認します。意味をなさなくなった場合は、その変数グループをオブジェクトにまとめる必要があることを示す良い兆候です。

対処
繰り返しデータがクラスのフィールドで構成される場合、「Extract Class」を使用してフィールドを独自のクラスに移動します。
メソッドのパラメーターで同じデータの塊が渡される場合は、「Introduce Parameter Object」を使用してそれらをクラスとして設定します。
一部のデータが他のメソッドに渡される場合、個々のフィールドだけではなく、データオブジェクト全体をメソッドに渡すことを検討してください。「Preserve Whole Object」がこれに役立ちます。
これらのフィールドで使用されるコードを見てください。このコードをデータクラスに移動することをお勧めします。

効果

  • ソースコードが理解しやすくなります。また、コードの構成が改善されます。特定のデータに対する操作は、コード全体で無計画に行われるのではなく、単一の場所に収集されるようになりました。
  • コードのサイズを小さくすることができます。

Object-Orientation Abusers

Object-Orientation Abusers(オブジェクト指向の乱用)
オブジェクト指向プログラミングの原則を不完全、不正確に適用することで発生するCode Smellです。

Switch Statements

Switch Statements

兆候と症状
複雑なswitch文またはif文の連続がこのCode Smellの兆候です。

問題の理由
switch/case演算子の比較的まれな使用は、オブジェクト指向コードの特徴の1つです。多くの場合、同じようなswitch文はプログラム内のさまざまな場所に散在します。新しい条件が追加されたら、すべてのスイッチコードを見つけて変更する必要があります。
経験則として、switch文が使われている場所ではポリモルフィズムを検討する必要があります。

対処

  • switch文を分離して適切なクラスに配置するには、「Extract Method」と「Move Method」が必要になる場合があります。

  • プログラムのランタイムモードが切り替えられるときなどにように、switch文がtype codeに基づいている場合は、「Replace Type Code with Subclasses」か、「Replace Type Code with State/Strategy」を使用します。

  • 継承構造を指定した後、「Replace Conditional with Polymorphism」を使用します。

  • 演算子にあまり多くの条件がなく、それらがすべて異なるパラメーターで同じメソッドを呼び出す場合、ポリモルフィズムはでは問題を解決できません。この場合、「Replace Parameter with Explicit Methods」の手法でそのメソッドを複数の小さなメソッドに分割し、それに応じてswitch文を変更できます。

  • 条件オプションの1つがnullの場合、「Introduce Null Object」を使用します。

効果

  • ソースコードの構成を改善することができます。

When to Ignore
以下のような場合はswitch文をリファクタリングする必要はありません。

  • switch文がシンプルなものである場合。
  • GoFのデザインパターンである Factory MethodAbstract Factory パターンを実装する場合。

Temporary Field

Temporary Field

兆候と症状
Temporary fieldsは、特定の状況下でのみ値を取得します(したがって、オブジェクトに必要です)。これらの状況以外では、それらは空です。

※Temporary Fieldに関してはこの記事の方が私はわかりやすかったので、兆候と症状のところはこの記事をベースに書いてみます。

Estimator
public class Estimator
{
    private readonly TimeSpan defaultEstimate;
    private IReadOnlyCollection<TimeSpan> durations;
    private TimeSpan average;
    private TimeSpan standardDeviation;

    public Estimator(TimeSpan defaultEstimate)
    {
        this.defaultEstimate = defaultEstimate;
    }

    public TimeSpan CalculateEstimate(
        IReadOnlyCollection<TimeSpan> durations)
    {
        if (durations == null)
            throw new ArgumentNullException(nameof(durations));

        if (durations.Count == 0)
            return this.defaultEstimate;

        this.durations = durations;
        this.CalculateAverage();
        this.CalculateStandardDeviation();

        var margin = TimeSpan.FromTicks(this.standardDeviation.Ticks * 3);
        return this.average + margin;
    }

    private void CalculateAverage()
    {
        this.average =
            TimeSpan.FromTicks(
                (long)this.durations.Average(ts => ts.Ticks));
    }

    private void CalculateStandardDeviation()
    {
        var variance =
            this.durations.Average(ts => 
                Math.Pow(
                    (ts - this.average).Ticks,
                    2));
        this.standardDeviation = 
            TimeSpan.FromTicks((long)Math.Sqrt(variance));
    }
}

上記EstimatorクラスのCalculateEstimateメソッド内でdurationsというフィールドを使用してます。同時に、CalculateAverageメソッド内ではaverageを使用しており、CalculateStandardDeviationメソッド内ではstandardDeviationを使用しています。これらのメソッドはCalculateEstimateメソッドで呼ばれているため、CalculateEstimateは、明示的にdurationsを使用し、さらに暗黙的にaverage, standardDeviationフィールドも使用しています。
averageとstandardDeviationはdurationsに依存しています。さらに、standardDeviationはaverageに依存しています。
ここで、CalculateEstimateメソッド内の処理の順番を以下のように変更したらどうなるでしょうか?

変更前
this.durations = durations;
this.CalculateAverage();
this.CalculateStandardDeviation();
変更後
this.durations = durations;
this.CalculateStandardDeviation();
this.CalculateAverage();

コンパイルは通ります。CalculateEstimateを実行してもExceptionは発生しません。しかし、想定外の結果になります。
このコードは理解しにくいだけではなく、脆弱です(処理の順番を入れ替えただけで破綻します)。さらに、スレッドセーフでもありません。

問題の理由
多くの場合、Temporary fieldsは、大量の入力を必要とするアルゴリズムで使用するために作成されます。そのため、プログラマーはメソッド内に多数のパラメーターを作成する代わりに、クラスの中にこのデータのフィールドを作成することにします。これらのフィールドはアルゴリズムでのみ使用され、それ以外では使用されません。
この種のコードは理解するのが難しいです。オブジェクトフィールドにデータが表示されるはずですが、何らかの理由でほとんど常に空です。
(フィールドに値を設定するメソッドが実行されるまではnullが設定されている)

対処
Temporary fieldとそれらを操作するすべてのコードは、「Extract Class」を介して別のクラスに入れることができます。つまり、メソッドオブジェクトを作成して、「Replace Method with Method Object」を実施します。

Introduce Null Object」を導入し、Temporary fieldの値の存在を確認するために使用された条件コードをそこに統合します。

効果

  • コードが明確になり、コードの構成が改善されます。

Refused Bequest

Refused Bequest(接続拒否)

兆候と症状
サブクラスがその親から継承したメソッドとプロパティの一部のみを使用する場合、クラス階層は歪んでいます。不要なメソッドは単純に未使用になるか、再定義されて例外が発生します。

問題の理由
コードを再利用したいというだけの理由でスーパークラスを作成した場合にこのCode Smellが発生します。スーパークラスとサブクラスの間には何の関連性もありません。

対処
継承に意味がなく、サブクラスにスーパークラスとの共通点がまったくない場合は、継承を削除して、「Replace Inheritance with Delegation」の技法を用います。
継承が適切な場合は、サブクラスの不要なフィールドとメソッドを取り除きます。サブクラスに必要なすべてのフィールドとメソッドを親クラスから抽出し、それらを新しいサブクラスに入れ、両方のクラスをそれから継承するように設定します(Extract Superclass)。

※参考:リスコフの置換原則

効果
コードが明確になり、クラス設計を改善します。
DogクラスがChairクラスから継承される理由を不思議に思う必要はもうありません(両方とも4本の脚を持っていますが)。

image.png

Alternative Classes with Different Interfaces

Alternative Classes with Different Interfaces

兆候と症状
異なるメソッド名を有する2つのクラスが同じ機能を実装しているような状況。

問題の理由
一方のクラスを作成したプログラマは、機能的に同等のクラスが既に存在することをおそらく知らなかったでしょう。

対処
各クラスの共通部分をインターフェースとして抽出してみてください:

  • すべてのクラスでメソッド名が同じになるように「Rename Method」を適用します。
  • Move Method」、「Add Parameter」、および「Parameterize Method」により、メソッドのシグネチャ(メソッド名、引数の数、引数の型、引数の順番)と実装を同じにします。
  • クラスの機能の一部のみが重複している場合は、「Extract Superclass」を使用してみてください。この場合、既存のクラスはサブクラスになります。
  • 使用するリファクタリング方法を決定して実装した後、クラスの1つを削除できる場合があります。

効果

  • 不要な重複コードを排除することができます。
  • コードの可読性が上がり、理解しやすくなります。まったく同じ機能を実現するクラスが2つある理由を推測する必要がなくなります。

Change Preventers

Change Preventers(変更を妨げるもの)
これらのCode Smellは、コードのある場所で何かを変更すると、他の場所でも多くの変更を行う必要があることを意味します。
プログラム開発ははるかに複雑になり、コストがかかるものになってしまいます。

Divergent Change

Divergent Change(変更の発散)

※Divergent Changeは、Shotgun Surgeryという別のCode Smellと似ていますが、Divergent ChangeとShotgun Surgeryは真逆のものです。
Divergent Changeは1つのクラスに多くの変更が加えられることです。Shotgun Surgeryは一つの変更が複数クラスに対して同時に行われることを指します。

兆候と症状
クラスに変更を加えると、多くの無関係なメソッドを変更する必要があります。たとえば、新しい製品タイプを追加する場合、製品の検索、表示、注文の方法を変更する必要があります。

問題の理由
このように修正箇所が発散する原因は、多くの場合、不十分なプログラム構造またはコピペプログラミングによるものです。

対処

  • Extract Class」を使ってクラスの振る舞いを別クラスに分割します。

  • 異なるクラスが同じ振る舞いを持っている場合、継承を利用してクラスを統合します(「Extract Superclass」と「Extract Subclass」)。

効果

  • コード構成の改善
  • 重複コードの削減

Shotgun Surgery

Shotgun Surgery(変更の分散)

※Shotgun Surgeryは、Divergent Changeという別のCode Smellと似ていますが、Shotgun SurgeryとDivergent Changeは真逆のものです。
Divergent Changeは1つのクラスに多くの変更が加えられることです。Shotgun Surgeryは一つの変更が複数クラスに対して同時に行われることを指します。

兆候と症状
変更を行う際、多くの異なるクラスに多くの小さな変更を加える必要がある場合。

問題の理由
単一の責任が多数のクラスに分割されたことが原因です。これは、Divergent Changeを熱心に適用した後に発生する可能性があります。

対処
Move Method」と「Move Field」を使用して、既存の様々なクラスに分散した振る舞いを、単一のクラスに移動します。これに適したクラスがない場合は、新しいクラスを作成してください。

コードを同じクラスに移動して元のクラスをほとんど空のままにした場合、「Inline Class」を介してこれらの冗長なクラスを削除してみてください。

効果

  • コード構成の改善
  • コード重複の削減
  • メンテナンス性の向上

Parallel Inheritance Hierarchies

Parallel Inheritance Hierarchies(パラレル継承)

兆候と症状
クラスのサブクラスを作成するたびに、別のクラスのサブクラスを作成する必要がある場合。

※正直この説明だけだとどういう状況なのかがよくわからなかった。
このサイトの方がわかりやすかったのでこちらをベースに訳してみました。

image.png

このサイトより抜粋

どうやら似たような継承関係を持つクラスが複数あるような状況で、新たなクラスを追加するともう一方の方にもクラスを追加する必要があるような状況らしい。

上の図で言うと、VehicleクラスがOperatorクラスを保有している(フィールドがある)ような状況で、Vehicleのサブクラスに新しいクラスを追加したいような場合にOperatorも追加しないといけないような状況のことを言っているらしい。

対処法も違うやり方が掲載されているのでご紹介

Defer identification of state variables pattern

Defer identification of state variables pattern(状態変数の識別延期パターン)

状態変数(図1でいうPoint)をサブクラスに識別させるパターン。
(サブクラスでの定義をDefer identification:遅延識別と言っている)
スーパークラスでアクセッサをabstractで定義しておいて、サブクラスでOverrideすることで、柔軟な設計ができるようになる。

【図1】
image.png
このサイトより抜粋

【図2】
image.png
このサイトより抜粋

Intelligent children pattern

Intelligent children pattern

Defer identification of state variables のより具体的な話。
スーパークラス同士をコンポジットさせるのではなく、サブクラス同士をコンポジットさせるパターン。

image.png
このサイトより抜粋

スーパークラスにOperatorを返すアクセッサをAbstractで定義しておいて、サブクラスでOperatorのサブクラスを返すようにOverrideすることで実現する。

image.png
このサイトより抜粋

問題の理由
階層が小さければ問題は発生しにくいですが、新しいクラスが追加されると、変更を加えることがだんだん難しくなります。

対処
※対処は元のサイト(sourcemaking)を訳しています。
上で紹介した「Intelligent children pattern」とは別の対処方法の話です。

2つの手順でParallel Inheritance Hierarchiesを排除できます。まず、ある階層のインスタンスが別の階層のインスタンスを参照するようにします。次に、「Move Method」と「Move Field」を使用して、参照先クラスの階層を削除します。

【リファクタリング前】
image.png
このサイトより抜粋

【リファクタリング後】
image.png
このサイトより抜粋

効果

  • 重複コードの排除
  • コード構成の改善

When to Ignore
Parallel Inheritance Hierarchiesは、プログラムアーキテクチャでさらに大きな混乱を避けるための方法の一つです。階層の重複を排除しようとしても醜いコードが生成される場合は、すべての変更を元に戻し、そのコードに慣れてください。

Dispensables

Dispensables
Dispensablesとは、コードをきれいで効率的で理解しやすくする上で必要のないもののことです。

Comments

Comments(コメント)

兆候と症状
メソッドが説明コメントで埋め尽くされている状態。

問題の理由
コメントは通常、プログラム作成者が自分のコードが直感的でも明白でもないことに気付いたときに、それを改善する目的で記載されます。そのような場合、コメントは、改善されるべき臭いのするコードに消臭剤を撒いて臭いを隠すようなものです。

もっともよいコメントはメソッドやクラスに良い名前を付けることです。

コメントがないと理解できないようなソースコードは、コメントが不要になるようにコード構造を変更してみてください。

対処

  • コメントが複雑な式を説明することを目的としている場合、式は「Extract Variable」を使用して理解可能な部分式に分割する必要があります。

  • コメントがコードのセクションを説明している場合、このセクションは「Extract Method」を介して別メソッドとして抽出できます。新しいメソッドの名前はコメントを参考にしましょう。

  • メソッドがすでに抽出されているが、メソッドが何をするのかを説明するためにコメントが必要な場合は、メソッド名をわかりやすいものにします。これには「Rename Method」を使用します。

  • システムが機能するために必要な状態に関するルールをアサートする必要がある場合は、「Introduce Assertion」を使用します。

効果

  • コードがより直感的で明確になります。

When to Ignore
時としてコメントは有用なものです。

  • 特定の方法で実装されている理由を説明するとき。
  • 複雑なアルゴリズムを説明するとき(アルゴリズムを簡素化する他のすべての方法が試され、不足しているとき)。

Duplicate Code

Duplicate Code(重複コード)

兆候と症状
ほとんど同じようなコードが見られる場合

問題の理由
通常、コードの重複は、複数のプログラマーが同じプログラムの異なる部分を同時に操作しているときに発生します。彼らはさまざまなタスクに取り組んでいるので、同僚が似たようなコードを既に書いていることに気付いていない可能性があります。

コードの特定の部分が異なって見えるが実際には同じジョブを実行する場合のように、より微妙な重複もあります。このような重複は、見つけて修正するのが難しい場合があります。

意図的に重複させている場合もあります。納期に追われているようなときで、既存のコードが”ほぼ正しく”要件を満たせる場合、初心者のプログラマーは、関連するコードをコピーして貼り付ける誘惑に抵抗できないかもしれません。また、場合によっては、単にプログラマーが整理するのをさぼるようなこともあります。

対処
同じクラスの2つ以上のメソッドで重複コードが見つかった場合、「Extract Method」を使用して新しいメソッドを作成し、両方の場所でその新しいメソッドを呼び出します。

  • 2つの同階層のサブクラスで重複コードを見つけた場合:

    • 両方のクラスに「Extract Method」を用いて、抽出したメソッドに対して「Pull Up Field」で使用されているフィールドを抽出します。
    • 重複コードがコンストラクタの内部に存在する場合、「Pull Up Constructor Body」を適用します。
    • 重複コードが似ているが完全には一致しない場合、「Form Template Method」を適用します。
    • 2つのメソッドが同じことを別のアルゴリズムで実現している場合、最適なアルゴリズムを選択し、「Substitute Algorithm」を適用します。
  • 重複コードが異なる二つのクラスで見つかった場合:

    • 対象のクラスが階層の一部ではない場合、「Extract Superclass」を適用して新しいスーパークラスを一つ作り、そのスーパークラスに重複コードを持たせます。
    • スーパークラスを作るのが難しい場合は、一方のクラスから「Extract Class」を使用してクラスを抽出し、もう一方のクラスは抽出したコンポーネントを使用します。
  • 多数の条件式が存在し、同じコードを実行する(条件のみが異なる)場合は、「Consolidate Conditional Expression」を使用してこれらの演算子を単一の条件にマージし、「Extract Method」を使用して、条件を簡単な別のメソッドに配置し、分かりやすいメソッド名を付けます。

  • 条件式のすべての分岐で同じコードが実行される場合:「Consolidate Duplicate Conditional Fragments」を使用して、条件ツリーの外側に同じコードを配置します。

効果

  • 重複コードをマージすることでコードの構造が簡潔になり、コードを短くすることができる。
  • 簡潔さ+短さ=単純化が容易で、サポートが安価なコード。

Lazy Class

Lazy Class(怠け者クラス)

兆候と症状
クラスを理解して維持するには、常に時間とコストがかかります。したがって、時間とコストをかける価値のないクラスは削除する必要があります。

問題の理由
おそらく、最初のうちはクラスはちゃんと機能するように設計されていました。しかし、いくつかのリファクタリングの後、クラスは途方もなく小さくなりました。

あるいは、決して成し遂げられなかった将来の開発作業をサポートするために設計されたのかもしれません。

対処
ほとんど役に立たないコンポーネントには、「Inline Class」を行う必要があります。
関数が少ないサブクラスの場合は、「Collapse Hierarchy」を試してください。

効果

  • コードサイズを小さくすることができる。
  • メンテナンス性の向上。

Data Class

Data Class(データクラス)

兆候と症状
データクラスは、フィールドとそれらへのアクセッサメソッド(getter/setter)のみを含むクラスを指します。これらは、他のクラスで使用される単なるデータのコンテナです。これらのクラスには追加機能は含まれておらず、所有するデータを独立して操作することはできません。

問題の理由
新しく作成されたクラスに含まれるパブリックフィールドの数が少ないことは普通のことです(getter/setterが少数の場合もあります)。しかし、オブジェクトの真の力は、オブジェクト内のデータに動作タイプや操作を含めることができることです。

対処

  • クラスにパブリックフィールドが含まれる場合は、「Encapsulate Field」を使用して直接アクセスできないようにし、getter/setterを介してのみデータにアクセスできるようにしましょう。

  • コレクション(配列など)に格納されているデータには、「Encapsulate Collection」を使用します。

  • データクラスを使用しているクライアントコードを確認します。その中に、データクラス自体に配置されるべき機能があった場合、「Move Method」と「Extract Method」を使用して、その機能をデータクラスに移行します。

よく考え抜かれたメソッドでクラスがいっぱいになったら、クラスデータへの過度に広範なアクセスを可能にするデータアクセスの古いメソッドを削除することができます。これには、「Remove Setting Method」と「Hide Method」が役立つ場合があります。

効果

  • コードが理解しやすくなり、構成が改善されます。特定のデータに対する操作は、コード全体で無計画に行われるのではなく、単一の場所に収集されるようになりますた。
  • クライアントコードの重複を見つけるのに役立ちます。

Dead Code

Dead Code(到達不能コード)

兆候と症状
使用されない変数、パラメーター、フィールド、メソッド、またはクラス。

問題の理由
ソフトウェアの要件が変更されたり、修正が加えられたりすると、古いコードをクリーンアップする時間がありませんでした。
このようなコードは、分岐の1つが(エラーまたは他の状況により)到達不能になったときに、複雑な条件で見つかる可能性もあります。

対処
Dead Codeを見つける最も簡単な方法は、優れたIDEを使用することです。
未使用のコードや不要なファイルを削除します。
クラスが不要な場合で、サブクラスまたはスーパークラスは使用される場合、「Inline Class」または「Collapse Hierarchy」を適用できます。
不要なパラメーターを削除するには、「Remove Parameter」を使用します。

効果

  • コードサイズの削減

Speculative Generality

Speculative Generality

兆候と症状
未使用のクラス、メソッド、フィールド、またはパラメーターの存在。

問題の理由
実装されない将来の機能をサポートするために、念のためにコードが作成されることがあります。その結果、コードの理解とサポートが難しくなります。

対処
使用されていない抽象クラスを削除するには、「Collapse Hierarchy」を試してください。

  • 不要な委譲処理は、「Inline Class」を介して排除しましょう。
  • 未使用のメソッドは「Inline Method」を使用して取り除きます。
  • 使用されていないパラメーターを持つメソッドは、「Remove Parameter」の助けを借りて見てください。
  • 未使用のフィールドは単純に削除できます。

効果

  • コードがスリムになります。

When to Ignore
フレームワークを開発している場合、フレームワークのユーザーがその機能を必要とする限り、フレームワーク自体で使用されていない機能を作成することは非常に合理的です。
要素を削除する前に、それらがテストクラスで使用されていないことも確認してください。これは、テストクラスが対象クラスから内部情報を取得したり、テスト関連の特別なアクションの実行に必要な場合に発生します。

Couplers

このグループのすべてのCode Smellは、クラス同士の結合度が高くなったり、結合を委譲(delegate)に置き換え過ぎた場合に何が起こるかを示しています。

Feature Envy

Feature Envy(機能の横恋慕)

兆候と症状
自クラス内のデータよりも、別オブジェクトのデータに頻繁にアクセスするメソッド。

問題の理由
このCode Smellは、フィールドがデータクラスに移動された後に発生する場合があります。この場合、データの操作もこのクラスに移動できます。

対処
基本的なルールとして、同時に変化するようなものは同じ場所に保管する必要があります。通常、データとこのデータを使用する機能は一緒に変更されます(ただし、例外はあります)。

  • メソッドを明らかに別の場所に移動する必要がある場合は、「Move Method」を使用します。
  • メソッドの一部のみが別オブジェクトのデータにアクセスする場合、「Extract Method」を使用して問題の部分を移動します。
  • メソッドが他のいくつかのクラスの機能を使用する場合、まず、最も頻繁に使用するデータがどのクラスのものなのかを判別します。次に、判別したクラスにそのメソッドを移動します。または、「Extract Method」を使用して、メソッドを複数の部分に分割し、異なるクラスの異なる場所に配置します。

効果

  • 重複コードの削減(データを操作するコードが適切な場所に配置されていれば)
  • コード構成の改善(データを処理するメソッドがデータの近くに配置されていれば)

When to Ignore
動作は、データを保持するクラスとそれに関連する振る舞いを意図的に分離している場合があります。こうすることで、動作を動的に変更できるという利点が得られます。
詳細はStrategy Design PatternVisitor Design Pattern、および他のデザインパターンを参照してください。

Inappropriate Intimacy

Inappropriate Intimacy(不適切な関係)

兆候と症状
別のクラスの内部フィールドとメソッドを使用するクラス。

問題の理由
一緒に行動することが多いクラスに注意してください。優れたクラスは、お互いのことをできる限り知らないようにする必要があります。このようなクラスは、保守と再利用が簡単です。

対処
最も簡単な解決策は、「Move Method」と「Move Field」を使用して、クラスの一部を、それを使用しているクラスに移動することです。しかし、これは移動前のクラスが本当にその部品を必要としない場合にのみ機能します。

効果

  • コード構成の改善
  • コードの保守性を再利用性の向上

Message Chains

Message Chains(メッセージの連鎖)

兆候と症状
$ a-> b()-> c()-> d()というような一連のメソッド呼び出し

問題の理由
Message Chainsは、クライアントが別のオブジェクトを要求し、そのオブジェクトがさらに別のオブジェクトを要求したときなどに発生します。このような関係の変更には、クライアントの変更が必要です。

対処
Message Chainsを削除するには、「Hide Delegate」を使用します。

Message Chainsの終端のオブジェクトが使用されている理由を考える方が良い場合があります。おそらく、この機能に「Extract Method」を使用し、「Move Method」を使用してチェーンの先頭に移動するのが理にかなっています。

効果

  • クラス間の依存関係を減らすことができます。
  • コードのバルクを削減できます。

When to Ignore
Hide Delegateしすぎた結果、実際に処理が行われる場所の確認が難しくなることがあります。言い方を変えると、Middle ManのCode Smellも避けてください。

Middle Man

Middle Man(仲介人)

兆候と症状
他クラスのメソッドを呼び出すだけのクラスがあるとしたら、そのクラスの存在意義は何でしょうか?

問題の理由
このCode Smellは、別のCode SmellであるMessage Chainsを熱心に排除した結果である可能性があります。

別の可能性としては、クラスの有用な処理が他のクラスに徐々に移動された結果である可能性があります。クラスは、委譲(delegate)以外の処理を行わない抜け殻として残存しています。

対処
クラスのメソッドが行っていることのほとんどが別のクラスへの処理の委譲(delegate)である場合、「Remove Middle Man」を行いましょう。

効果

  • Less bulky code.

Incomplete Library Class

Incomplete Library Class

兆候と症状
遅かれ早かれ、Libraryはユーザーのニーズを満たすことをやめます。ライブラリは読み取り専用であるため、問題の唯一の解決策であるライブラリの変更は、多くの場合不可能です。

問題の理由
ライブラリの作成者は、必要な機能を提供していないか、それらの実装を拒否しています。

対処

効果
コードの重複を削減します(独自のライブラリをゼロから作成する代わりに、既存のライブラリを流用できます)。

When to Ignore
ライブラリコードが変更された場合、拡張ライブラリに対して追加の作業が発生する可能性があります。

関連するリファクタリング技法

Composing Methods

Composing Methods

Extract Method

Extract Method
まとまった処理をメソッドとして切り出すこと。

関連するCode Smell

Inline Method

Inline Method

メソッドがメソッドを呼び出すような関係になっていてそれが可読性を低下させているようであればメソッドのインライン化を検討しましょう。

リファクタリング前
class PizzaDelivery {
  // ...
  int getRating() {
    return moreThanFiveLateDeliveries() ? 2 : 1;
  }
  boolean moreThanFiveLateDeliveries() {
    return numberOfLateDeliveries > 5;
  }
}
リファクタリング後
class PizzaDelivery {
  // ...
  int getRating() {
    return numberOfLateDeliveries > 5 ? 2 : 1;
  }
}

関連するCode Smell

Extract Variable

Extract Variable
複雑で理解が難しい処理がある場合、処理を細かく分割し、わかりやすい変数名を付けるようにしましょう。

リファクタリング前
void renderBanner() {
  if ((platform.toUpperCase().indexOf("MAC") > -1) &&
       (browser.toUpperCase().indexOf("IE") > -1) &&
        wasInitialized() && resize > 0 )
  {
    // do something
  }
}
リファクタリング後
void renderBanner() {
  final boolean isMacOs = platform.toUpperCase().indexOf("MAC") > -1;
  final boolean isIE = browser.toUpperCase().indexOf("IE") > -1;
  final boolean wasResized = resize > 0;

  if (isMacOs && isIE && wasInitialized() && wasResized) {
    // do something
  }
}

関連するCode Smell

Inline Temp

Inline Temp
※関連するCode Smellがなかったためリンクのみ。

Replace Temp with Query

Replace Temp with Query
変数の初期化を別のメソッドとして切り出すこと。
わかりやすいメソッド名をつけることで、変数の意図を明確にできます。また、コードの重複を排除できる。

関連するCode Smell

Split Temporary Variable

Split Temporary Variable
※関連するCode Smellがなかったためリンクのみ。

Remove Assignments to Parameters

Remove Assignments to Parameters
※関連するCode Smellがなかったためリンクのみ。

Replace Method with Method Object

Replace Method with Method Object
関連する変数と処理をクラスとして抽出すること。

関連するCode Smell

Substitute Algorithm

Substitute Algorithm(アルゴリズムの置き換え)
メソッドのシグニチャは変えずに本体となるアルゴリズムを新しいものに置き換えましょう。

リファクタリング前
String foundPerson(String[] people){
  for (int i = 0; i < people.length; i++) {
    if (people[i].equals("Don")){
      return "Don";
    }
    if (people[i].equals("John")){
      return "John";
    }
    if (people[i].equals("Kent")){
      return "Kent";
    }
  }
  return "";
}
リファクタリング後
String foundPerson(String[] people){
  List candidates =
    Arrays.asList(new String[] {"Don", "John", "Kent"});
  for (int i=0; i < people.length; i++) {
    if (candidates.contains(people[i])) {
      return people[i];
    }
  }
  return "";
}

関連するCode Smell

Moving Features between Objects

Moving Features between Objects

Move Method

Move Method
あるクラス内のメソッドが自クラスよりも他クラスから呼び出されることの方が多い場合、そのメソッドを最も使用頻度が高いクラスに移動しましょう。

関連するCode Smell

Move Field

Move Field
クラス内のフィールドが自クラスよりも他クラスから使われることが多い場合、そのフィールドを使用頻度が高いクラスに移動し、参照先を移動先クラスに変更しましょう。

関連するCode Smell

Extract Class

Extract Class
1つのクラスに複数の責任を持たせるとコードが複雑になることがあります。その場合は、新しいクラスを作成し、関連する機能を担当するフィールドとメソッドをその中に配置します。

このリファクタリング方法は、単一責任の原則を守るのに役立ちます。単一責任の原則を守ることでクラスはより明確で理解しやすいものになります。

関連するCode Smell

Inline Class

Inline Class

ほとんど何もせず、何の責任もなく、責任が追加される計画もないようなクラスは、その機能を既存の適切なクラスに移動しましょう。

関連するCode Smell

Hide Delegate

Hide Delegate

オブジェクトAを介してオブジェクトBを取得したのち、オブジェクトBのメソッドを呼び出しているような場合、クラスAに新しいメソッドを追加し、そのメソッドがオブジェクトBのメソッドを呼び出すような形にしましょう(=委譲(delegate))。こうすることで、クライアントはオブジェクトBの詳細について知る必要がなくなり、依存性を排除できます。

AとBは下の図でいうと、
- A:Person
- B:Department

【リファクタリング前】
image.png
このページより抜粋

【リファクタリング後】
image.png
このページより抜粋

関連するCode Smell

Remove Middle Man

Remove Middle Man
多くのメソッドが行っていることが他クラスへの処理の委譲である場合、それらのメソッドを削除し、クライアントクラスから直接、委譲先のメソッドを呼び出すようにしましょう。

【リファクタリング前】
image.png
このページより抜粋

【リファクタリング後】
image.png
このページより抜粋

関連するCode Smell

Introduce Foreign Method

Introduce Foreign Method

ライブラリ内のユーティリティクラスには必要なメソッドが含まれておらず、そのクラスにはメソッドを追加することもできません。
その場合、クライアントクラスにメソッドを追加し、ユーティリティクラスのオブジェクトを引数として渡します。

リファクタリング前
class Report {
  // ...
  void sendReport() {
    Date nextDay = new Date(previousEnd.getYear(),
      previousEnd.getMonth(), previousEnd.getDate() + 1);
    // ...
  }
}
リファクタリング後
class Report {
  // ...
  void sendReport() {
    Date newStart = nextDay(previousEnd);
    // ...
  }
  private static Date nextDay(Date arg) {
    return new Date(arg.getYear(), arg.getMonth(), arg.getDate() + 1);
  }
}

関連するCode Smell

Introduce Local Extension

Introduce Local Extension

ライブラリのユーティリティクラスには、必要なメソッドが含まれていません。ただし、これらのメソッドをクラスに追加することはできません。
メソッドを含む新しいクラスを作成し、それをユーティリティクラスの子またはラッパーにします。

【リファクタリング前】
image.png

【リファクタリング後】
image.png

関連するCode Smell
- Incomplete Library Class

Organizing Data

Organizing Data

Self Encapsulate Field

Self Encapsulate Field
※関連するCode Smellがなかったためリンクのみ。

Replace Data Value with Object

Replace Data Value with Object
プリミティブ型で定義されたデータフィールドとその振る舞いを別の新しいクラスとして切り出し、それを元のクラスに保有させます。

※参考:ValueObject
Replace Data Value with Objectを理解するためにDDDのValueObjectが参考になると思ったので、ValueObjectを解説しているサイトのリンクを張っておきます。

関連するCode Smell

Change Value to Reference

Change Value to Reference
※関連するCode Smellがなかったためリンクのみ。

Change Reference to Value

Change Reference to Value
※関連するCode Smellがなかったためリンクのみ。

Replace Array with Object

Replace Array with Object

様々なデータ型が1つの配列に含まれている場合、配列をオブジェクトに変換し、型が異なるデータは別のフィールドに持たせましょう。

リファクタリング前
String[] row = new String[2];
row[0] = "Liverpool";
row[1] = "15";
リファクタリング後
Performance row = new Performance();
row.setName("Liverpool");
row.setWins("15");

関連するCode Smell

Duplicate Observed Data

Duplicate Observed Data
GUIを担当するクラスにドメイン(=業務)に関するデータが含まれているような場合、データを別のクラスに分離し、ドメインクラスとGUI間の接続と同期を確保することをお勧めします。
こうすることで、プレゼンテーションレイヤーとビジネスロジックを分離することができ、プログラムを理解しやすくすることができます。

関連するCode Smell

Change Unidirectional Association to Bidirectional

Change Unidirectional Association to Bidirectional
※関連するCode Smellがなかったためリンクのみ。

Change Bidirectional Association to Unidirectional

Change Bidirectional Association to Unidirectional
クラス同士が相互に依存しているが、一方のクラスは他方の機能を使用していない場合、未使用の関連を削除します。

【リファクタリング前】
image.png
このページより抜粋

【リファクタリング後】
image.png
このページより抜粋

関連するCode Smell

Replace Magic Number with Symbolic Constant

Replace Magic Number with Symbolic Constant
※関連するCode Smellがなかったためリンクのみ。

Encapsulate Field

Encapsulate Field
publicなフィールドはprivateに変更してsetter/getterからのアクセスのみを許可するようにしましょう。
カプセル化はオブジェクト指向の基本です。
データに対して必要な処理(値のチェックなど)をsetter/getterに任せることで、データを利用する側ではその正当性を意識する必要がなくなります。

関連するCode Smell

Encapsulate Collection

Encapsulate Collection
ListやSetなどのようなCollectionをカプセル化する場合、getterはread onlyなオブジェクトを返すようにしましょう。
また、Collectionに要素を追加するためのaddメソッドやdeleteメソッドを用意しましょう。

※なぜこんなことをする必要があるのかわからない人はimmutableという概念について知っておきましょう。参考になりそうなサイトのリンクを下に張っておきます。

このページでは、immutableにするためにgetCourcesメソッドはUnmodifiableSetをreturnしています。UnmodifiableSetにaddやdeleteメソッドで要素を追加・削除しようとすると、実行時エラー(UnsuportedOperationException)になります。

Effective Java という本の中では、defensive copyというやり方が紹介されています。
これは、上で紹介したUnmodifiableSetをreturnするのではなく、以下のようにして、参照先の異なるオブジェクトをreturnする手法です。
これであれば、呼び出し元でaddやdeleteメソッドを呼び出すことが可能であり、かつ、オブジェクトの参照先が異なるので元データに影響を与えることもありません。

Sample.java

import java.util.HashSet;
import java.util.Set;

class Test {
    private Set<String> setTest = new HashSet<>();
    public Set<String> getCources() {
        return new HashSet<>(setTest);    // ここで元のHashSetと同じ値を持つ別のHashSetを生成してそれをreturn
    }
}

【immutableの参考】

ちなみに、Javaでimmutable objectの一番身近な例はStringです。

関連するCode Smell

Replace Type Code with Class

Replace Type Code with Class
クラスにType Code(※)が含まれていて、かつそれがプログラムの振る舞いに何も影響を与えないのであれば、それをクラスとして切り出し、それを値として使うのではなくオブジェクトとして扱いましょう。
(Enumを使うときれいにはまりそうですね)

※Type Codeについて
Type Codeは個別のデータ型ではありません。エンティティに登録できる値が一連の数字または文字列に制限されているような場合に発生します。多くの場合、これらの特定の数字と文字列には定数を介してわかりやすい名前が付けられています。

関連するCode Smell

Replace Type Code with Subclasses

Replace Type Code with Subclasses
プログラムの振る舞いに直接影響を与えるType Codeが存在する場合、各値をサブクラスとして切り出し、関連する振る舞いをサブクラスに移動します。そして、条件分岐などでコントロールしていた部分をポリモルフィズムで実現するように書き換えましょう。

関連するCode Smell

Replace Type Code with State/Strategy

Replace Type Code with State/Strategy
クラスの中にType Codeがあり、それがプログラムの振る舞いに影響を与えるような場合で、サブクラスとして切り出すのが難しい場合、Type Codeを状態を表すオブジェクト(state object)に置き換えます。フィールドの値をType Codeに置き換える必要がある場合、別の状態オブジェクトをクラスに注入します。

※GoFのデザインパターンのStateパターンの話だと理解しました。
参考

関連するCode Smell

Replace Subclass with Fields

Replace Subclass with Fields
※関連するCode Smellがなかったためリンクのみ。

Simplifying Conditional Expressions

Simplifying Conditional Expressions

Decompose Conditional

Decompose Conditional(条件文の分解)
複雑な分岐条件をメソッドとして切り出すこと。

関連するCode Smell

Consolidate Conditional Expression

Consolidate Conditional Expression
複数の異なる条件分岐で同じ結果を返すような処理は一つの条件に統合しましょう。

リファクタリング前
double disabilityAmount() {
  if (seniority < 2) {
    return 0;
  }
  if (monthsDisabled > 12) {
    return 0;
  }
  if (isPartTime) {
    return 0;
  }
}
リファクタリング後
double disabilityAmount() {
  if (isNotEligableForDisability()) {
    return 0;
  }
}

関連するCode Smell

Consolidate Duplicate Conditional Fragments

Consolidate Duplicate Conditional Fragments
条件分岐内で重複コードが存在する場合はその処理を条件分岐の外に配置しましょう。

リファクタリング前
if (isSpecialDeal()) {
  total = price * 0.95;
  send();
}
else {
  total = price * 0.98;
  send();
}
リファクタリング後
if (isSpecialDeal()) {
  total = price * 0.95;
}
else {
  total = price * 0.98;
}
send();

関連するCode Smell

Remove Control Flag

Remove Control Flag
※関連するCode Smellがなかったためリンクのみ。

Replace Nested Conditional with Guard Clauses

Replace Nested Conditional with Guard Clauses
※関連するCode Smellがなかったためリンクのみ。

Replace Conditional with Polymorphism

Replace Conditional with Polymorphism
switch文で条件を指定しながら処理を振り分けるような処理は、処理に対応するサブクラスを作成して呼び出し元はポリモルフィズムで実現することを検討しましょう(※)。

※Effective Java で”戦略enumパターン”という名前で紹介されていた技法も参考になると思われるので、それに関する記事をご紹介

関連するCode Smell

Introduce Null Object

Introduce Null Object
あるメソッドがnullを返すようなことがある場合、呼び出し元でnullチェックを行う必要があります。
nullの代わりにデフォルトの振る舞いを実装したオブジェクトを返すことを検討しましょう。

関連するCode Smell

Introduce Assertion

Introduce Assertion
コードが正しく動作するのに必要な前提条件がある場合、assert()を使用して事前チェックするようにしましょう。

関連するCode Smell

Simplifying Method Calls

Simplifying Method Calls

Rename Method

Rename Method
どのような処理を行ってくれるのかがわかるようなメソッド名を付けるように心がけましょう。

関連するCode Smell

Add Parameter

Add Parameter
処理を行うのに必要なデータが足りない場合は引数で渡すようにしましょう。

関連するCode Smell

Remove Parameter

Remove Parameter
メソッド本体で使われていないようなパラメータは削除しましょう。

関連するCode Smell

Separate Query from Modifier

Separate Query from Modifier
※関連するCode Smellがなかったためリンクのみ。

Parameterize Method

Parameterize Method
似たような処理を行うメソッドが複数存在するような場合はメソッドの統合を検討しましょう。
例えば、5%値を上昇させるメソッドと、10%値を上昇させる2つのメソッドが存在する場合、上昇させる値(ここでは5や10)を引数で受け取る一つのメソッドに統合する場合がこれに当たります。

【リファクタリング前】
image.png
このページより抜粋

【リファクタリング後】
image.png
このページより抜粋

関連するCode Smell

Replace Parameter with Explicit Methods

Replace Parameter with Explicit Methods
メソッド内が複数の処理に分割され、各処理は条件に応じて実行されるような場合、各処理を行うメソッドを作り、それを呼び出すことを検討しましょう。

関連するCode Smell

Preserve Whole Object

Preserve Whole Object
メソッドの引数をばらばらに渡すのではなく、一つのオブジェクトにまとめて渡す。
引数となるオブジェクトにわかりやすい名前を付けることでコードが読みやすくなる。
また、引数が増えた場合でも、オブジェクトにデータを追加するだけでよくなり、呼び出し側の影響を少なくすることができる。

リファクタリング前
int low = daysTempRange.getLow();
int high = daysTempRange.getHigh();
boolean withinPlan = plan.withinRange(low, high);
リファクタリング後
boolean withinPlan = plan.withinRange(daysTempRange);

関連するCode Smell

Replace Parameter with Method Call

Replace Parameter with Method Call
引数として値を渡す際、事前に何かしらの処理が必要なのであれば、引数ではなくメソッドの中で処理してしまうことで引数を減らすことを検討しましょう。

リファクタリング前
int basePrice = quantity * itemPrice;
double seasonDiscount = this.getSeasonalDiscount();
double fees = this.getFees();
double finalPrice = discountedPrice(basePrice, seasonDiscount, fees);
リファクタリング後
int basePrice = quantity * itemPrice;
double finalPrice = discountedPrice(basePrice);

関連するCode Smell

Introduce Parameter Object

Introduce Parameter Object
パラメータオブジェクトの導入。
関連する値を1つのオブジェクトにまとめることでコードが読みやすくする。
開始日時と終了日時を期間というクラスにまとめるなど。

関連するCode Smell

Remove Setting Method

Remove Setting Method

データクラス内のデータがインスタンス生成時にのみ設定される場合、setterは必要ないので削除しましょう。

関連するCode Smell

Hide Method

Hide Method

自クラス内やサブクラスでのみ使用され、他クラスからは使用されないメソッドの可視性はprivateかprotectedにしましょう。

関連するCode Smell

Replace Constructor with Factory Method

Replace Constructor with Factory Method
※関連するCode Smellがなかったためリンクのみ。

Replace Error Code with Exception

Replace Error Code with Exception
※関連するCode Smellがなかったためリンクのみ。

Replace Exception with Test

Replace Exception with Test
※関連するCode Smellがなかったためリンクのみ。

Dealing with Generalisation

Dealing with Generalisation

Pull Up Field

Pull Up Field
2つのクラスに同じフィールドが存在する場合はそのフィールドをスーパークラスに移動しましょう。

関連するCode Smell

Pull Up Method

Pull Up Method
※関連するCode Smellがなかったためリンクのみ。

Pull Up Constructor Body

Pull Up Constructor Body
ほとんど同一のコードがコンストラクタに存在する場合、スーパークラスにコンストラクタを定義してサブクラスの重複コードをスーパークラスに移動し、サブクラスのコンストラクタからスーパークラスのコンストラクタを呼び出すようにしましょう。

関連するCode Smell

Push Down Method

Push Down Method
※関連するCode Smellがなかったためリンクのみ。

Push Down Field

Push Down Field
※関連するCode Smellがなかったためリンクのみ。

Extract Subclass

Extract Subclass

特定の場合にのみ使用される機能がクラスにある場合、サブクラスを作成してその機能を使用します。

注意すべき点として、サブクラスを作ることが必ずしもコードの複雑性を回避する手段になるとは限りません。継承は使いどころを間違えると大変なことになるのでコンポジションを使うべきところと明確に分けて考えましょう。

※継承とコンポジションの違いについてはこの辺が参考になると思います。

関連するCode Smell

Extract Superclass

Extract Superclass
2つのクラスの共通部分(フィールドやメソッド)をスーパークラスとして再定義しましょう。

※継承は必ずしも最適解ではありません。使いどころを間違えるとクラス設計がいびつなものになってしまいます。
先の項目にも書いた継承を使うべきか委譲(delegate)を使うべきかをきちんと判断した上で継承を使うようにしましょう。

・・・例えば、ゲームのスーパーマリオに出てくるファイアーマリオを実装する場合、普通のマリオをスーパークラスにしてファイアーマリオをそのサブクラスとして実装するのはおそらく違和感ないです。ファイヤーマリオの実装はおそらく「火を吐く」という部分だけを実装すれば済むでしょう。
しかし、クッパを普通のマリオやファイアーマリオのサブクラスとして実装するのはおそらく後々厄介なことになります。
確かにクッパはジャンプをするし、火も吐くのでマリオとの共通部分が多く、もしかしたらマリオを継承することで実装を減らせるかもしれません。
しかし、マリオの実装を変えるとクッパの振る舞いに影響を与えかねません。
スーパークラスとサブクラスは結びつきが強いため、実装を変更するとお互いの振る舞いに影響を与えることがあります。本当に継承を使うのが適切なのかを考えてから継承を使うようにしましょう。

関連するCode Smell

Extract Interface

Extract Interface

リンク先の英語をほぼ理解できなかったのですが、、、雰囲気的に以下のようなことを言っていると思いました。

この技法はコードの重複を排除するようなものではありません。
状況によって役割・振る舞いが違う場合にこの技法は有用です。
(例えば、DB接続する機能があったとして、ローカル環境ではStub、結合テスト環境では実際にDB接続するクラスとして振舞ってほしいような状況のことを言っているのかなと想像しました)

関連するCode Smell

Collapse Hierarchy

Collapse Hierarchy
サブクラスとスーパークラスにほとんど差がないようであればスーパークラスに統合しましょう。

関連するCode Smell

Form Template Method

Form Template Method
サブクラスに似たようなステップ、順序で実行するアルゴリズムが実装されている場合、アルゴリズムの構造および同一ステップをスーパークラスに移動し、異なるステップの実装をサブクラスから排除します。

【リファクタリング前】

このページより抜粋

【リファクタリング後】

このページより抜粋

関連するCode Smell

Replace Inheritance with Delegation

Replace Inheritance with Delegation
スーパークラスの一部しか使っていないようなサブクラスがある場合、(あるいは、スーパークラスのデータをサブクラスに引き継ぐのが不適切な場合)
サブクラスとして定義したクラスをスーパークラスのフィールドとして定義し、サブクラスのメソッドをスーパークラスに委譲(delegate)しましょう。さらに、継承関係を排除しましょう。

関連するCode Smell

Replace Delegation with Inheritance

Replace Delegation with Inheritance
他クラスのメソッドを呼び出すだけのシンプルなメソッド(委譲(delegate))を多く実装しているような場合、委譲を継承に置き換えることを検討しましょう。

関連するCode Smell

予備知識

委譲(delegate)

委譲と継承の違いはこれを見ればわかると思います。

※余談:転送と委譲について
正確に言うと、上で紹介したサイトで委譲と言っているのは厳密には「転送」と言います。コンポジット+転送です。でも世の中のほとんどの人はこの「転送」を委譲(delegate)と呼んでいます。

参考:

その他の委譲の参考

単一責任の原則

単一責任の原則(SRP)

リスコフの置換原則

親子関係にあるクラス構造において、親が実現できていることは子でも必ず実現されてなければならない。という原則。

参考

その他Code Smellに関する参考

42
35
2

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
42
35