新米エンジニアによるリファクタリングのすすめ-後編-

はじめに

この記事は『新米エンジニアによるリファクタリングのすすめ-前編-』の続きです。
前編を読まれていない方は、それを先に読むとより内容がわかりやすいと思います。

リファクタリングの必要性を感じるコード

早速ですが、怪しい香りのするコードについて具体的に解説していきます。

再代入を繰り返す変数

def sample(text)
  tmp = text.delete('円')
  tmp = tmp.delete(',')
  tmp = tmp.to_i
  tmp = tmp > 500 ? tmp : tmp * 1.5
  # なんらかの複雑な処理
  tmp.to_s
end

再代入は変数の意味が無闇に変更されるため、コードが理解しづらくなります。

またコードを修正するために、いつ値が変更されたのかを特定する必要があり、修正に多くの時間を必要とします。
さらに、読み手を不用に混乱させてしまうため、意図しないバグを生じさせてしまうといったデメリットもあります。
パフォーマンス面やメモリ面で特別な理由がない限り、変数の再代入は行わず、目的に応じて別の変数を用意するようにしましょう。

重複したコード

class UsersController
  def index
    user = User.find(1)
    #
    # ユーザー関連の情報を取得する処理
    #
    # indexメソッド独自の処理
  end

  def edit
    user = User.find(1)
    #
    # ユーザー関連の情報を取得する処理(indexメソッドと同じ)
    #
    # editメソッド独自の処理
  end
end

重複コードは、修正する際に漏れなく全ての重複箇所に修正を加える必要があり、手間にもなる上、修正漏れが生じやすいです。
また、コードを読む際に似たコードの微妙な差異に気を配る必要もあります。
以上のように重複コードは面倒が多くなるので、できるだけ避けましょう。
上記のサンプルのように同じクラス内に重複するコードが存在する場合は、重複箇所をメソッドとして抽出し、そのメソッドを元のメソッドから呼ぶようにしましょう。

プリミティブ型への執着

class UsersController
  def update
    phone_number = params[:phone_number]
    # '-'で区切られた電話番号を生成
    phone_number_with_hyphen = format_with_hyphen(phone_number)
    User.find(1).update!(phone_number: phone_number_with_hyphen)
  end

  private

  def format_with_hyphen(phone_number)
    #
    # 市外局番に応じて電話番号を'-'区切りにする
    # 
  end
end

class SampleService
  def sample
    phone_number = params[:phone_number]
    # '-'で区切られた電話番号を生成
    phone_number_with_hyphen = format_with_hyphen(phone_number)
    # なんらかの処理
  end

  private

  def format_with_hyphen(phone_number)
    #
    # 市外局番に応じて電話番号を'-'区切りにする
    # 
  end 
end

まずプリミティブ型とは、boolean, int, stringといったプログラミング言語が標準で用意しているデータ型のことです。
そして「プリミティブ型への執着」とは、自身が作成しているプログラムの中で頻発する金額や電話番号といったデータをクラス化せずに、プリミティブ型として扱うことです。

確かにプリミティブ型だけでプログラムを作成することは可能です。

しかし、その書き方ではデータ及びそれと関係が深いロジックを一つのクラスにまとめることができず、重複コードを生みやすい構造になってしまいます。

例えば上記の例では電話番号が単なる文字列データとして扱われ、
「市外局番に応じて電話番号を'-'区切りにする」
処理がそれぞれの箇所で書かれていて、DRYな状態とは言えません。
ここで、電話番号クラスを作成することでこの処理を1箇所にまとめることができます。
また、電話番号クラスをinitilizeする際に、電話番号としてありえない桁数が引数として渡された際にエラーを発生させるようにすれば、予期せぬバグを生むリスクを下げることもできます。

不要なコメント

class Money
  attr_accessor :value
  #
  # 初期化
  # 不正な値の場合エラー
  #
  def initialize(value)
    raise ArgumentError if value < 0

    @value = value
  end

  # 処理の具体的な内容(Aを〜して、Bを〜して...といったもの)
  def sample
    # なんらかの複雑な処理
  end
end

優れたコメントは、コードを読む上での大きな助けとなり、理解にかかる時間を短縮してくれますが、不要なコメントは逆にコードリーディングにかかる時間を増やし、開発生産性を落とします。

コメントの目的は、書き手の意図を読み手に知らせることである。(*1)

(*1)『リーダブルコード』から引用

必要なコメントだけを正確に残すことは非常に難しいことですが、上記の考えに基づくことである程度コメントすべきこととすべきでないことの区別がつくようになります。

例えば上記サンプルコードにある

# 初期化

このコメントは全く必要のないコメントです。
initializeそのものを説明しているだけで、書き手の意図が全く伝わりません。
また、

# 処理の具体的な内容

といったコメントも相応しくありません。
処理の具体的内容ではなく、そのコードを書いているときの自分の考え、例えば
「なぜ、処理に書いてるやり方を選択したのか」
「なぜ、他のやり方を選択しなかったのか」
といったことを書くべきです。

他にも、コメントの書き方にもコツがあり、例えば

# 不正な値の場合エラー

というコメントは、「負の値の場合エラー」と書いた方が伝わる情報量が多いです。
できるだけ少ないコメントで多くの情報を伝えることを意識すると良いコメントが書けると思います。

まとめると、コメントを残す際に

• 書き手の意図を知らせること
• 少ない記述量で多くの情報を詳細に伝えること

を意識すると、より良いコメントが残せると思います。

終わりに

今回はリファクタリングの必要性を感じるコードについて、いくつかのパターンを説明しました。
自分の書いたコードで悲しい思いをする人が1人でも少なくなるよう、日々リファクタリングしましょう。
コードには出来るだけいい人生を送って欲しいですしね。

それでは、良いリファクタリングライフを！！

参考文献

『リーダブルコード』
『良いコード/悪いコードで学ぶ設計入門 ―保守しやすい 成長し続けるコードの書き方』
『リファクタリング 既存のコードを安全に改善する（第2版）』