技術メモ

技術メモ

ラフなメモ

リファクタリングを読んだ【仕掛り】

点検読書

全体として何に関する本か?何がどのように詳しく書かれているか?

リファクタリングのガイドブック。
リファクタリングとは、ソフトウェアの外部の振る舞いを保ったままで、内部の構造を改善していく作業。
リファクタリングをすることで以前に書いたコードの設計が向上する。

対象の読者

ソフトウェア開発を行っている職業プログラマが対象

著者はどのような構成で概念や知識を展開しているか?

カタログ部分は最初はざっと何が書かれているか把握しておけばよく、実際に適用する際に詳細を把握するのが良い。


メモ

第1, 2章

第3章

  • リファクタリングを行うタイミング
    • 重複したコード(Duplicated Code)
    • 長すぎるメソッド(Long Method)
    • 巨大なクラス(Large Class)
    • 長すぎるパラメータリスト(Long Parameter List)
    • 変更の偏り(Divergent Change)
      • 一つのクラスが別々の理由で何度も変更される状況
    • 変更の分散(Shogun Surgery)
      • 一つの変更であちこちのクラスが少しずつ書き換わる場合
    • 特性の横恋慕(Feature Envy)
      • あるメソッドが自分のクラスよりも他のクラスに興味を持つ場合
      • Strategyパターン
      • Visitorパターン
    • データの群れ(Data Clumps)
    • 基本データ型への執着(Primitive Obsession)
    • Switch文(Switch Statement)
    • パラレル継承(Parallel Inheritance Hierarchies)
      • 「変更の偏り」の特殊なパターン
      • 新たなサブクラスを定義するたびに、別の継承ツリーにもサブクラスを定義しなければならない状況
    • 怠け者クラス(Lazy Class)
    • 疑わしき一般化(Speculative Gereratity)
    • 一時的属性(Temporary Field)
    • メッセージの連鎖(Message Chains)
      • メッセージの過剰な連鎖
    • 仲介人(Middle Man)
    • 不適切な関係(Inappopriate Intimacy)
    • クラスのインターフェース不一致(Alternative Classes with Different Interfaces)
    • 未熟なクラスライブラリ(Incomplete Library Class)
    • データクラス(Data Class)
      • データクラスは子どものようなもの
    • 相続拒否(Refused Bequest)
      • 親クラスの属性や操作を必要としていない場合
    • コメント(Comments)
      • コメントが消臭剤
  • 単体テストプログラマの生産性を向上するために行うもの

第6章:メソッドの構成

メソッドを適切にパッケージ化されたコードとして構成すること

メソッドの抽出

「ひとまとめにできるコードの断片がある」→「コードの断片をメソッドにして、目的を表すような名前をつける」

メソッドのインライン化

「メソッドの本体が、名前をつけて呼ぶまでもなく明らかである」→メソッド本体を呼び出し元にインライン化して、メソッドを除去する
int getRating() {
    return (moreThanFiveLateDeliveries()) ? 2 : 1;
}
boolean moreThanFiveLateDeliveries() {
  return _numberOfLateDeriveries > 5;
}

moreThanFiveLateDeliveries() で何をやりたいかはメソッド内部の _numberOfLateDeriveries > 5 を見れば明らかであるので、わざわざメソッドに切り出す必要がない。

int getRating() {
    return (_numberOfLateDeriveries > 5) ? 2 : 1;
}

メソッド本体を見れば明らかの場合は、メソッドで間接化されている分だけ冗長になっているという場合。これはあまりないかな?

一時変数のインライン化

「簡単な式によって一度だけ代入される一時変数があり、それが他のリファクタリングの障害になっている。」→その一時変数への参照をすべて式で置き換える。
double basePrice = anOrder.baseprice();
return (basePrice > 1000);

basePrice は冗長なだけなので、式で置き換える。

return anOrder.baseprice() > 1000;

この例だと anOrder.baseprice() > 1000 の条件すら anOrderカプセル化して anOrder.isXXX() とかになる可能性ある?
あとは baseprice() の呼び出しにコストがかかる場合は、やはり一時変数においておくとかは十分考えられる。

問い合わせによる一時変数の置き換え

式の結果を保持するために一時変数を使っている。
→式をメソッドに抽出する。一時変数へのすべての参照を新たなメソッドに置き換える。これにより新たなメソッドが他のメソッドでも使えるようになる。
double basePrice = _quantity * _itemPrice;
if (basePrice() > 1000) {
  // ある処理
} else {
  // 別の処理
}

_quantity * _itemPrice のロジックをメソッドに切り出します。

if (basePrice() > 1000) {
  // ある処理
} else {
  // 別の処理
}

...

double getBasePrice() {
  return _quantity * _itemPrice;
}

これはよくありそう。

... ローカル変数は抽出を難しくします。可能な限り、多くの変数を問い合わせで置き換えます。...

説明用変数の導入

「複雑な式がある」→その式の結果または部分的な結果を、その目的を説明する名前をつけた一時変数に代入する。

式が複雑で読みにくいときに可読性を高めるためのテクニック。一時的に使うための変数なので final をつけておくこと。

通常、「説明用変数の導入」を適用するよりも「メソッドの抽出」を適用する方が、手間がかかるとは思いません。...どのようなときに「説明用変数の導入」を適用するのでしょうか。それは、「メソッドの抽出」のほうが手間がかかるときです。...

一時変数の分離

「何回も代入される一時変数があるが、それは、ループ変数でも一時変数を集める変数でもない」→代入ごとに別の一時変数にわかる。

これは本当にそうで、意味が異なる一時変数を使い回すのは業務上のプログラミングでは基本的にNGという認識である。

パラメータへの代入の除去

「パラメータへの代入が行われている」→一時変数を使う。

以下のようにメソッド内でのパラメータへの代入は意味がない。混乱のもとである。

public class Param {

    public static void main(String[] args) {

        Date d1 = new Date(2019-1900, 4, 13);
        nextDateReplace(d1);
        System.out.println(d1);

    }

    private static void nextDateReplace(Date arg) {
        arg = new Date(arg.getYear(), arg.getMonth(), arg.getDay() + 1);
        System.out.println(arg);
    }
}
// Thu May 02 00:00:00 JST 2019
// Mon May 13 00:00:00 JST 2019

この nextDateReplace(Date arg) はバグってますね...

ドメインオブジェクトによるメソッドの置き換え

「長いメソッドで、「メソッドの抽出」を適用できないようなローカル変数の使い方をしている。」
→メソッド自身をオブジェクトとし、すべてのローカル変数をそのオブジェクトのフィールドとする。そうすれば、そのメソッドを同じオブジェクト中のメソッド群に分解できる。

メソッドの分解を困難にするのはローカル変数

たとえば以下のようなクラスとメソッドがあったとしてこれをリファクタリングする場合

public class Account {

    int gamma(int inputVal, int quality, int yearToDate) {
        int importantValue1 = (inputVal * quality) + delta();
        int importantValue2 = (inputVal * yearToDate) + 100;
        if ((yearToDate - importantValue1) > 100) {
            importantValue2 -= 20;
        }
        int importantValue3 = importantValue2 * 7;
        return importantValue3 - 2 * importantValue1;
    }

    int delta() {
        return 1;
    }
}

もともとAccountクラスにあったロジックをGammaクラスを切り出して、もとのメソッドはメソッドオブジェクトに委譲するように書き換える。

public class Account {

    int gamma(int inputVal, int quality, int yearToDate) {
        return new Gamma(this, inputVal, quality, yearToDate).compute();
    }

    int delta() {
        return 1;
    }
}
public class Gamma {
    private final Account account;
    private int inputVal;
    private int quality;
    private int yearToDate;
    private int importantValue1;
    private int importantValue2;
    private int importantValue3;

    public Gamma(Account source, int inputVal, int quality, int yearToDate) {
        this.account = source;
        this.inputVal = inputVal;
        this.quality = quality;
        this.yearToDate = yearToDate;
    }

    int compute() {
        importantValue1 = (inputVal * quality) + account.delta();
        importantValue2 = (inputVal * yearToDate) + 100;
        if ((yearToDate - importantValue1) > 100) {
            importantValue2 -= 20;
        }
        importantValue3 = importantValue2 * 7;
        return importantValue3 - 2 * importantValue1;
    }
}

アルゴリズムの取り替え

アルゴリズムをよりわかりやすいものに置き換えたい」→メソッドの本体を新たなアルゴリズムで置き換える。

第7章:オブジェクト間での特性の移動

メソッドの移動

「あるクラスでメソッドが定義されているが、現在または将来において、そのクラスの特性よりも他クラスの特定の方が、そのメソッドを使ったり、そのメソッドから使われたりすることが多い」
→同様の本体を持つ新たなメソッドを、それを最も多用するクラスに作成する。元のメソッドは単純な委譲とするか、またはまるごと取り除く。

クラスの振る舞いが多すぎる場合や、クラス間でのやり取りが多く、結合度が高すぎる場合にメソッドを移動します。メソッドを移動することで、クラスは単純になり、結果として、ひとまとまりの責務をすっきりとした実装に収めることができます。

以下の場合に overdraftCharge() のメソッドを別出しできるかどうか考えます。

class Account {
  double overdraftCharge() {
    if (_type.isPremium()) {
      double result = 10;
      if (_daysOverdrawn > 7) result += (_daysOverdrawn - 7) * 0.85;
      return result;
    } else {
      return _daysOverdrawn * 1.75;
    }
  }

  double bankCharge() {
    double result = 4.5;
    if (_daysOverdrawn > 0) result += overdraftCharge();
    return result;
  }
  private AccountType _type;
  private int _daysOverdrawn;
}

メソッド内部で利用しているインスタンス変数をメソッドのパラメータとして渡すようにして、メソッドごと別クラス (AccountType.java) に移動します。

class Account {
  double overdraftCharge(int daysOverdrawn) {
    return _type.overdraftCharge(daysOverdrawn);
  }
  double bankCharge() {
    double result = 4.5;
    if (_daysOverdrawn > 0) result += _type.overdraftCharge(daysOverdrawn);
    return result;
  }
  private AccountType _type;
  private int _daysOverdrawn;
}
class AccountType {
  double overdraftCharge(int daysOverdrawn) {
    if (isPremium()) {
      double result = 10;
      if (daysOverdrawn > 7) result += (account.getDaysOverdrawn() - 7) * 0.85;
      return result;
    } else {
      return account.getDaysOverdrawn() * 1.75;
    }
  }
}

Accountクラスでインスタンス変数として使われていた _daysOverdrawn はAccontTypeクラスからは account.getDaysOverdrawn() と参照することになります。
移したり戻したり、いろいろ試行錯誤が必要そうです。

フィールドの移動

「あるクラスに定義されているフィールドが、現在または将来において、定義しているクラスよりも、他のクラスから使われることが多い」→移動先のクラスに新たなフィールドを作って、その利用側をすべて変更する。

クラス間で状態や振る舞いを移動するのは、リファクタリングの本質です。

使われることが多い場所にフィールドを移しましょう。ということ。

class Account {
  private AccountType _type;
  private double _interestRate;

  double interestForAmountDays(double amount, int days) {
    return _interestRate * amount * days / 365;
  }
}

上記の _interestRateAccountType クラスに移動させることを考えます。

class Account {
  private AccountType _type;

  double interestForAmountDays(double amount, int days) {
    return _type.getInterestRate() * amount * days / 365;
  }
}
class AccountType {
  private double _interestRate;

  voud setInterestRate(double arg) {
    _interestRate = arg;
  }

  double getInterestRate() {
    return _interestRate;
  }
}

これだけ見るとシンプルなので大して難しさはないが...。

クラスの抽出

「2つのクラスでなされるべき作業を1つのクラスで行っている」→クラスを新たに作って、適切なフィールドとメソッドを元のクラスからそこに移動する。

データとメソッドの一部をまとめて別のクラスにできそうであれば、それは良い目安です。他にも、データの一部がよく同時に変更されたり、特にお互いに依存する関係にあれば、それも良い目安になります。

3章で出てきた、「巨大なクラス」「変更の偏り」「変更の分散」などに該当しているのではないでしょうか。

public class Person {

    private String _name;
    private String _officeNumber;

    public String getName() {
        return _name;
    }
    public void setName(String name) {
        this._name = name;
    }
    public String getOfficeNumber() {
        return _officeNumber;
    }
    public void setOfficeNumber(String officeNumber) {
        this._officeNumber = officeNumber;
    }
}

Personクラスから電話番号に関する振る舞いを分離させることで、Personクラスがすっきりしました。元のgetter, setterはTelephoneNumberクラスに委譲しています。

public class TelephoneNumber {

    private String _areaCode;
    private String _number;

    public void setNumber(String arg) {
        this._number = arg;
    }

    public String getNumber() {
        return _number;
    }

    public String getAreaCode() {
        return _areaCode;
    }

    public void setAreaCode(String arg) {
        this._areaCode = arg;
    }

    public String getTelephoneNumber() {
        return "(" + _areaCode + ")" + _number;
    }
}
public class Person {

    private String _name;
    private TelephoneNumber _officeTeTelephone = new TelephoneNumber();

    public String getName() {
        return _name;
    }
    public void setName(String name) {
        this._name = name;
    }
    public String getOfficeAreaCode() {
        return _officeTeTelephone.getAreaCode();
    }
    public void setOfficeAreaCode(String arg) {
        _officeTeTelephone.setAreaCode(arg);
    }
    public String getOfficeNumber() {
        return _officeTeTelephone.getNumber();
    }
    public void setOfficeNumber(String arg) {
        _officeTeTelephone.setNumber(arg);
    }
    public String getTelephoneNumber() {
        return _officeTeTelephone.getTelephoneNumber();
    }
}

クラスのインライン化

「そのクラスがやっていることは大したことでない」→別のクラスにその特性を移動して、それを削除する。

「クラスの抽出」の逆パターン。

あるリファクタリングに伴ってクラスから他の責務も移動され、責務がほとんど残らなくなってしまう

委譲の隠蔽

「クライアントがあるオブジェクトの委譲クラスを呼び出している」→サーバにメソッドを作って委譲を隠す。

f:id:tutuz:20190414121953p:plain

class Person {
  Department _department;

  publib Department getDepartment() {
    return _department;
  }

  public void setDepartment(Department arg) {
    _department = arg;
  }
}
class Department {
  private String _chargeCode;
  private Person _manager;

  public Department(Person manager) {
    _manager = manager;
  }

  public Person getManager() {
    return _manager;
  }
}

上記において、クライアントがある人の manager を知りたい場合は maganer = john.getDepartment().getManager(); とする必要があります。この時、Departmentクラスをクライアントから隠蔽することで、結合度を低くすることができます。
以下のようにPersonクラスに委譲メソッドを作成することでクライアントからは maganer = john.getManager(); とするだけでよくなります。

クライアントが知っておくべき情報が少なくなってうれしい感じです。

カプセル化とは、オブジェクトが持つシステムの他の部分についての知識を減らす必要があることを意味します。ものごとが変化しても、変更を知らせる必要のあるオブジェクトがより少なければ、変更を加えやすくなります。

class Person {
  public Person getManager() {
    return _department.getManager();
  }
}

仲介人の除去

「クラスがやっていることは単純な委譲だけである」→クライアントに委譲オブジェクトを直接呼ばせる。

「委譲の隠蔽」の逆。

外部メソッドの導入

「利用中のサーバクラスにメソッドを追加する必要があるが、そのクラスを変更できない」→クライアントクラスに、サーバクラスのインスタンスを第一引数に取るメソッドを作る。

本来はサーバメソッドに追加すればよいだけであるが、サーバのメソッドは変更できない状態にあるので、クライアント側でなんとかしないといけない場合。クライアント側にロジックを追加するのだが、他の場所にも同じロジックを追加しないといけない可能性があるため、メソッドとして切り出しておく。ということである。

Date newStart = new Date(PreviousEnd.getYear(), PreviousEnd.getMonth(), PreviousEnd.getDay() + 1);

以下のように外部メソッド化する。

Date newStart = new Date(nextDay(PreviousEnd));

private static Date nextDay(Date arg) {
  return new Date(arg.getYear(), arg.getMonth(), arg.getDay() + 1);
}

局所的拡張の導入

「利用中のサーバクラスにメソッドをいくつか追加する必要があるが、クラスを変更できない」→それらの追加されるメソッドを備えた新たなクラスを作る。この拡張クラスは、元のクラスのサブクラスまたはラッパーである。

局所的拡張は別個のクラスですが、それはサブタイプとして元のクラスを拡張します。したがって、元のクラスでできることにはすべて対応した上で、さらに特定が加えられることになります。

  • ラッパーとするかサブクラスとするか

サブクラスを使う場合

class MfDateSub extends Date {
  public MfDateSub(String dateString) {
    super(dateString);
  }

  public MfDateSub(Date arg) {
    super(arg.getTime());
  }

  Date nextDay() {
    return new Date(getYear(), getMonth(), getDay() + 1);
  }
}

上記の場合、クライアント側の実装は以下のようになるのではないか。

class Client {

        MfDateSub sub = new MfDateSub(PreviousEnd);
        Date newStart = sub.nextDay();

}

Dateクラスの加算ロジックが間違っている気がするのだが、そこは一旦スルーする。

第8章:データの再編成

自己カプセル化フィールド

「フィールドを直接アクセスしているが、そのフィールドとの結合関係が煩わしくなってきた」→そのフィールドに対するgetメソッドとsetメソッドを作って、そのだけを使ってアクセスするように変更する。

やり方が決まるまではの最初の手として、直接アクセスを使っておくのがいいと思います。プログラムが汚くなり始めたら、間接アクセスに切り替えます。

private int _low, _high;
boolean isInclude(int arg) {
  return _low <= arg && arg <= _high;
}

これをgetter(), setter()を使って以下のように書き換える。

private int _low, _high;
boolean isInclude(int arg) {
  return getLow() <= arg && arg <= getHigh();
}

int getLow() {
  return _low;
}
int getHigh() {
  return _high;
}

基本的にフィールドアクセスは、アクセッサを使ったほうが望ましいと考えているのだか、どうだろうか。

オブジェクトによるデータ値の置き換え

「追加のデータや振る舞いが必要なデータ項目がある」→そのデータ項目をオブジェクトに変える。
class Order {
  private String _customer;

  public Order(String customer) {
    _customer = customer;
  }
  public String getCustomer() {
    return _customer;
  }
  public void setCustomer(String arg) {
    _customer = arg;
  }
}

クライアントの実装を以下を想定している。

private static int numberOfOrderFor(List<Order> orders, String customer) {
  int result = 0;
  for (Order order : orders) {
    if (order.getCustomer().equals(customer)) result++;
  }
  return result;
}

ここでOrderクラスに定義されているCustomerに関する情報をCustomerクラスとして切り出すことを考えます。

class Customer {
  private final String _name;
  public Customer(String name) {
    this._name = name;
  }
  public String getName() {
    return _name;
  }
}
class Order {
  private Customer _customer;
  public Order(String customerName) {
    _customer = new Customer(customerName);
  }
  public String getCustomerName() {
    return _customer.getName();
  }
  public void setCustomer(String customerName) {
    _customer = new Customer(customerName);
  }
}

上のCustomerクラスにクレジットの信用限度や住所のような項目を加えることですが、今はできません。それは、Orderオブジェクトが値オブジェクトとして扱われているからです。OrderオブジェクトはそれぞれにCustomerオブジェクトを保持しています。

値から参照への変更

「同じインスタンスが多数存在するクラスがある。それらを1つのオブジェクトに置き換えたい」→そのオブジェクトを参照オブジェクトに変える。
  • 参照オブジェクト
    • 顧客とか勘定といったもので、実世界における1個のオブジェクトを表しており、それらが同じものかどうかを調べるには、オブジェクト識別が用いられる。
  • 値オブジェクト
    • 日付、お金のようなもの。それ自身のデータ値によって定義される。

各注文は概念的には同じ顧客に対するものであっても、それぞれが自分のCustomerオブジェクトを持っています。 参照オブジェクトとして扱われているのか値オブジェクトとして扱われているのかの差がわからなかったが、この説明と以下のコードで意味が明らかになった。

private static int numberOfOrderFor(List<Order> orders, String customer) {
  int result = 0;
  for (Order order : orders) {
    if (order.getCustomer().equals(customer)) result++;
  }
  return result;
}

「Factory Methodによるコンストラクタの置き換え」を適用します。

class Customer {
  private Customer(String name) {
    _name = name;
  }

  public static Customer create(String name) {
    return new Customer(name);
  }
}
class Order {
  public Order(String customer) {
    _customer = Customer.create(customer);
  }
}

次にCustomerオブジェクトを要求された都度生成するのか、前もって生成するのか決める。以下は前もって生成しておく場合の実装でアプリケーション開始時に使用されるCustomerオブジェクトをロードする。Springなどを使う場合は予めDIコンテナにインスタンスを生成しておくことになる。

public class Customer {

    private static Map<String, Customer> _instances = new HashMap<>();
    private final String _name;

    static void loadCustomers() {
        new Customer("A").store();
        new Customer("B").store();
        new Customer("C").store();
    }

    private void store() {
        _instances.put(this.getName(), this);
    }

    public static Customer getNamed(String name) {
        return _instances.get(name);
    }

    public Customer(String name) {
        this._name = name;
    }

    public String getName() {
        return _name;
    }

}

オブジェクトを参照するときに同一の概念のオブジェクトが同一のオブジェクトを指しているかは意識しないといけない。

参照から値への変更

「小さくて、不変で、コントロールが煩わしい参照オブジェクトがある」→値オブジェクトに変える。

「値から参照への変更」の逆。

値オブジェクトの重要な性質は、不変であること

オブジェクトによる配列の置き換え

「配列の各要素が、それぞれ異なる意味を持っている」→その配列を、要素ごとに対応したフィールドを持つオブジェクトに置き換える。

このパターンはあまり見たことがない。配列のそれぞれのindexに異なる意味を持つオブジェクトを格納することがある??

観察されるデータの複製

単方向関連の双方向への変更

双方向関連の単方向への変更

シンボリック定数によるマジックナンバーの置き換え

フィールドのカプセル化

コレクションのカプセル化

データクラスによるレコードの置き換え

クラスによるタイプコードの置き換え

サブクラスによるタイプコードの置き換え

State/Strategyによるタイプコードの置き換え

フィールドによるサブクラスの置き換え