Subscribed unsubscribe Subscribe Subscribe

ジャバ・ザ・ハットリの日記

日本→シンガポール→ベルリンへと家族と共に流れ着き、ベルリンのスタートアップで働くソフトウェアエンジニアの日記

Rubyのリファクタリングでイケてないコードを美しいオブジェクト指向設計のコードへ改良するための方法 - その3

前回からの続き。
Rubyのリファクタリングでオブジェクト指向設計に沿った美しいコードになるまでの方法を書いた。

  1. 「イケてない」から「マシ」にするためのリファクタリング
  2. 「マシ」から「いいね」にするためのリファクタリング
  3. 「いいね」から「スゲーいいね」にするためのリファクタリング

今回は3.の「いいね」から「スゲーいいね」にするためのリファクタリング

「いいね」から「スゲーいいね」にするためのリファクタリング

前回までの変更点を反映した全体像

class OrdersReport
  def initialize(orders, date_range)
    @orders = orders
    @date_range = date_range
  end

  def total_sales_within_date_range
    orders_within_range.map(&:amount).inject(0) do |sum, amount|
      sum + amount
    end
  end

  private

  def orders_within_range
    @orders.select do |order|
      order.placed_between?(@date_range)
    end
  end
end

class DateRange < Struct.new(:start_date, :end_date)
  def include?(date)
    (start_date..end_date).cover? date
  end
end

class Order < OpenStruct
  def placed_between?(date_range)
    date_range.include?(placed_at)
  end
end


この中でとても読みにくい箇所があって、それはtotal_sales_within_date_rangeの辺り。
【変更前】

  def total_sales_within_date_range
    orders_within_range.map(&:amount).inject(0) do |sum, amount|
      sum + amount
    end
  end

mapしてamountがあって、それをinjectして初期値は0で、、、ってややこしすぎ。

ここはパブリックメソッドであり、クラス外からも参照される箇所になる。そうした外からも参照される箇所は極力シンプルにするようにすべき。誰も複雑なモノを呼びたくない。パブリックメソッドをシンプルで究極に可読性を上げることで、再利用性が高まる。
その下のprivateメソッドは多少複雑でも構わない。
パブリックメソッドをどのぐらいシンプルにするべきか、は「読んだら一瞬で分かるぐらい」がベスト。具体的にはこんな感じ。

【変更後】

  def total_sales_within_date_range
    total_sales(orders_within_range)
  end

これでもう一目瞭然。total_sales(総売上)はorders_within_range(範囲内のオーダ)から計算する、と。変更前に比べて脳への負荷が1%ぐらいで済む。

この変更にともなってtotal_salesという名のprivateメソッドに処理を移行させる。

  private

  def total_sales(orders)
    orders.map(&:amount).inject(0) do |sum, amount|
      sum + amount
    end
  end

ここにまだ一工夫ができる。
injectのちょっとした用法で:+を使うことで1行で表現できる。慣れている人にとってはお馴染みの方法で可読性がさらに上がる。

【変更後】

  def total_sales(orders)
    orders.map(&:amount).inject(0, :+)
  end

なんとたったの1行で表現できる。:+の詳しい解説はこことか分かりやすい。
ちょっとわかりにくいけど非常に便利なinjectメソッド - 勉強日記
もう:+はそのまま覚えておいても損はない。

これでリファクタリングは終了。これまでの全てのリファクタリングを反映させた全体像がこれ。
【完成したコード】

class OrdersReport
  def initialize(orders, date_range)
    @orders = orders
    @date_range = date_range
  end

  def total_sales_within_date_range
    total_sales(orders_within_range)
  end

  private

  def total_sales(orders)
    orders.map(&:amount).inject(0, :+)
  end

  def orders_within_range
    @orders.select { |order| order.placed_between?(@date_range) }
  end
end

class DateRange < Struct.new(:start_date, :end_date)
  def include?(date)
    (start_date..end_date).cover? date
  end
end

class Order < OpenStruct
  def placed_between?(date_range)
    date_range.include?(placed_at)
  end
end

もう最初のコードと比べると可読性、単一責任、再利用性、疎結合、と全ての要素が満たされて格段に良くなっているのが分かる。ほとんどのメソッド内の行数が1行。(2行なのはinitializeのみ)気持ちいいぐらいに読みやすい。

【リファクタリング前のイケてないコード】

class OrdersReport
  def initialize(orders, start_date, end_date)
    @orders = orders
    @start_date = start_date
    @end_date = end_date
  end

  def total_sales_within_date_range
    orders_within_range = []
    @orders.each do |order|
      if order.placed_at >= @start_date && order.placed_at <= @end_date
        orders_within_range << order
      end
    end

    sum = 0
    orders_within_range.each do |order|
      sum += order.amount
    end
    sum

  end
end

class Order < OpenStruct
end

最初のイケてなかった頃の変更前と変更後とを比較した。

変更前 変更後
可読性 読めても時間かかる スッキリ読める
責任 1メソッドに複数責任 キレイに単一責任
再利用性 ほぼ無理 容易に再利用可能
結合 密結合 疎結合

要素毎に再確認すると凄まじいぐらいに改善されていることが分かる。

今回の例にしたコードだけを見ると「1回しか呼んでない引数にわざわざStructなんか定義して、それ必要か?」とか思われそうだが、それはこのコード例だけを見ているからだ。これがある程度の規模のプロジェクトの一部だと仮定すると、より分かりやすくなる。その規模になれば必ず誰かがどこかで再利用する。その際に変更前と変更後のコードと、どちらがよりプロジェクトにとって、メンテナンスが容易で再利用がしやすいか?と考えれば明らかに変更後の方になる。

「期間内の総売上を出力するコードを書いてくれ」と頼まれて、そのままそれだけを書いてしまったのが変更前のコード。
「期間内の総売上の出力が必要でかつ、後で誰かがこのコードをメンテナンスして、また一部を再利用することがあるな」と想定して作られたのが変更後のコードになる。誰でも後者の方の考えを持つエンジニアと一緒に働きたいはず。

ということでこれで「Rubyのリファクタリングでイケてないコードを美しいオブジェクト指向設計へ改良するための全解説」は終わり。
 
 
実はここに書いている内容はほとんど「Practical Object-Oriented Design in Ruby」という本の受け売り。

Practical Object-Oriented Design in Ruby: An Agile Primer (Addison-Wesley Professional Ruby)

Practical Object-Oriented Design in Ruby: An Agile Primer (Addison-Wesley Professional Ruby)

もしここに書いたようなオブジェクト指向設計やリファクタリングについて「もっと深く知りたい」とお考えであれば、本書を読めば、この記事の100倍は納得感が得られると思う。イケてるコードを書きたい全てのエンジニアにとって「買っても絶対に損は無い本」と断言できるオススメの良書。この本についての詳しい書評はこちらに書いた。
tango-ruby.hatenablog.com



tango-ruby.hatenablog.com

tango-ruby.hatenablog.com

tango-ruby.hatenablog.com