ぽしゃけを飲みながら Ruby のコードをリファクタリングしてみた

ぽしゃけを飲みながら Rubyでリファクタリングをやってみよう を読んでいたんですが、無性にリファクタリングしたくなってきたので勢いでリファクタリングしました。

リファクタリング

class Purchaser
  attr_reader :section, :purchased_at

  def initialize(section:, purchased_at:)
    @section = section # 購入者の区分(こども、おとな、シニア)
    @purchased_at = purchased_at
  end
end

class Sales
  def initialize(purchasers, start_date, end_date)
    @purchasers = purchasers
    @start_date = start_date
    @end_date = end_date
  end

  def calc
    # start_dateからend_dateの期間内に購入されたデータを集める
    target_purchasers = []
    @purchasers.each do |purchaser|
      target_purchasers << purchaser if purchaser.purchased_at >= @start_date && purchaser.purchased_at <= @end_date
    end

    # 集めたデータの購入金額の合計を算出する
    sales = 0
    target_purchasers.each do |purchaser|
      # 購入者の区分(こども、おとな、シニア)によって購入金額を決定する
      sales += case purchaser.section
      when :child
        1000
      when :adult
        1800
      when :senior
        1100
      end
    end

    sales
  end
end

元記事のリファクタリング

Section = Struct.new(:sym, :charge)
SECTION = [
  Section.new(:child, 1000),
  Section.new(:adult, 1800),
  Section.new(:senior, 1100)
].each_with_object({}){ |section, h| h[section.sym] = section }.freeze

class Purchaser
  attr_reader :section, :purchased_at

  def initialize(section:, purchased_at:)
    @section = SECTION[section]
    @purchased_at = purchased_at
  end
end

class Sales
  def initialize(purchasers, start_date, end_date)
    @purchasers = purchasers
    @start_date = start_date
    @end_date = end_date
  end

  def calc
    purchasers_within_date.inject(0) { |sum, purchaser| sum + purchaser.section.charge }
  end
  
  private
    def purchasers_within_date
      @purchasers.filter_map do |purchaser|
        purchaser if (@start_date..@end_date).cover?(purchaser.purchased_at)
      end
    end
end

わたしのリファクタリング

class Purchaser < Struct.new(:section, :purchased_at, keyword_init: true)
  def sale
    case section
    when :child
      1000
    when :adult
      1800
    when :senior
      1100
    end
  end
end

class Sales < Struct.new(:purchasers, :start_date, :end_date)
  def calc
    # start_dateからend_dateの期間内に購入されたデータを集める
    target_purchasers = purchasers
      .each_with_object(data_range)
      .filter_map { |purchaser, range| purchaser if range.cover?(purchaser.purchased_at) }
      # 集めたデータの購入金額の合計を算出する
      .map(&:sale)
      .sum
  end

  private

  def data_range
    (start_date..end_date)
  end
end

所感

リファクタリング後はかなりすっきりしたコードになりましたね。
元の記事と比較して

  • Struct を継承するようにしてみた
  • sale の計算を Purchaser 側に寄せた
  • #calc はいい感じにチェーンするようにしてみた

のあたりを重点的にリファクタリングしてみました。
ぶっちゃけリファクタリング前のコードとわたしがリファクタリングしたコードはかなり挙動が違うんですがまあテストが通ってるからええかな…という感じです。
で、別に元記事のリファクタリングしたコードが悪い!!!とかそういうことでは全然なくてリファクタリングも人によって全然違うなーという意味での記事になります。
と、いうかぶっちゃけただリファクタリングしたかっただけです!!! まあ Ruby だといろいろな書き方ができるのでこういうやり方もありかな〜〜〜って感じの内容になります。
[元記事はいいことがいっぱい書いてある}(https://zenn.dev/hogucc/articles/e63ff0ec2d5c057ba020)のでぜひ読んでみましょう!!! リファクタリングは楽しいぞ!!!!!!