読者です 読者をやめる 読者になる 読者になる

はむはむエンジニアぶろぐ

このブログのコンセプトは"ハッキングの為なら愛する家族を傷つけることをいとわない" 自分にとってエンジニアリングは "手段ではなく生きる目的" である

リファクタリングコンテスト in Rubyに投稿してみた

f:id:secret_hamuhamu:20150501050313p:plain
ForkwellJobsで、リファクタリングコンテスト in Rubyというものが開催されています。
リファクタリングコンテスト in Ruby TOO

あなたがリファクタリングしたコードの { Before → After } を投稿しよう!
今までにリファクタリングを行ったRubyのコードのBeforeとAfterを投稿してください。
豪華審査員による厳正な審査によって選ばれた方には、下記の商品を贈呈します!

審査員の方々が、そうそうたる顔ぶれなので受賞すればお墨付きというか間違いなく良いコードを書いていると自慢できます!!
ということで、普段ほとんどRuby書かないのだけれども投稿してみました!

ちなみに、こちらが私の投稿です。

環境構成

  • ruby 2.0.0p481


Before

用意されているお題の一つ 【Refactor Me】if elseで挑戦してみた。

0 <= age or raise

if 0 <= age && age <= 2
  'baby'
elsif 3 <= age && age <= 6
  'little child'
elsif 7 <= age && age <= 12
  'child'
elsif 13 <= age && age <= 18
  'youth'
else
  'adult'
end


After

ちょっと長いけど、私が書いたAfterのコードです。
GitHubにも掲載しています。if_else.rb

class AgeCategory

  def self.name
    self::NAME
  end

  def self.fromTo
    self::LOWER_LIMIT..self::UPPER_LIMIT
  end

end

class Baby < AgeCategory
  NAME = 'baby'
  LOWER_LIMIT = 0
  UPPER_LIMIT = 2
end

class LittleChild < AgeCategory
  NAME = 'little child'
  LOWER_LIMIT = 3
  UPPER_LIMIT = 6
end

class Child < AgeCategory
  NAME = 'child'
  LOWER_LIMIT = 7
  UPPER_LIMIT = 12
end

class Youth < AgeCategory
  NAME = 'youth'
  LOWER_LIMIT = 13
  UPPER_LIMIT = 18
end

class Adult < AgeCategory
  NAME = 'adult'
  LOWER_LIMIT = 13
  UPPER_LIMIT = Float::INFINITY
end

class CategorizeByAge

  def initialize(age)
    0 <= age or raise
    @age = age
  end

  def get

    case @age
    when Baby.fromTo
      Baby.name
    when LittleChild.fromTo
      LittleChild::NAME
    when Child.fromTo
      Child.name
    when Youth.fromTo
      Youth.name
    when Adult.fromTo
      Adult.name
    end
  end

end

require 'test/unit'
class CategorizeByAgeTest < Test::Unit::TestCase

  data(
    '0〜2才の間_下限'   => ['baby',         0],
    '0〜2才の間_上限'   => ['baby',         2],
    '3〜6才の間_下限'   => ['little child', 3],
    '3〜6才の間_上限'   => ['little child', 6],
    '7〜12才の間_下限'  => ['child',        7],
    '7〜12才の間_上限'  => ['child',        12],
    '13〜18才の間_下限' => ['youth',        13],
    '13〜18才の間_上限' => ['youth',        18],
    '19才以上_下限'     => ['adult',        19],
  )
  def test_年齢に応じた区分を返すこと(data)
    expected, age = data
    byAge = CategorizeByAge.new(age)
    assert_equal(expected, byAge.get())
  end

  def test_年齢が負数であれば例外を返すこと
    assert_raise(RuntimeError) { CategorizeByAge.new(-1) }
  end

end


リファクタリングポイント

Beforeのifelseを見た瞬間、Strategyパターンだなと思いStrategyパターンで実装しました。

まず、そもそもテストがないのでテストを書いてからリファクタリングすることにしました。
Beforeが先にあるのでテストファーストではないですが、仕様を書いてred-green-リファクタリングを小さいステップで繰り返すTDD的アプローチ。
テストがあって適切な単位でコミットしていれば、どんな修正をしても怖くないです。
みんな、テストを書こう!!ヽ(^o^)丿

データとロジックを固めるのは原則なので、 baby やら little child やらに関連するデータとロジックを集めてクラス化していきました。
BabyクラスやLittleChildクラスがそうですね。
Beforeからしか仕様が分からないので、適切な抽象名なのかわからないけど。
クラスに分けることで、文字列リテラルやマジックナンバーを撲滅できます。

そんな感じで、リファクタリングすればAfterになりました。

ちょっと心残りなのが、 NAMELOWER_LIMIT といった定数の実装強制できていないこと。
AgeCategoryクラスの子クラスは、強制したい実装がされてないと例外を投げる仕様にしたかった。
定数じゃなくてメソッドの強制ならできるみたい。
メソッドの override を強制する

わざわざ、定数をメソッドにして実装強制すると、かえって複雑になりそうな気がしたので止めた。

私、Ruby初心者なのでもっと良い書き方あるよって人は是非、GitHubにコメントやPR頂ければと思います。

まとめ

Ruby楽しい!!

おすすめの本

実践テスト駆動開発 (Object Oriented SELECTION)

実践テスト駆動開発 (Object Oriented SELECTION)

レガシーコード改善ガイド (Object Oriented SELECTION)

レガシーコード改善ガイド (Object Oriented SELECTION)

  • 作者: マイケル・C・フェザーズ,ウルシステムズ株式会社,平澤章,越智典子,稲葉信之,田村友彦,小堀真義
  • 出版社/メーカー: 翔泳社
  • 発売日: 2009/07/14
  • メディア: 大型本
  • 購入: 45人 クリック: 673回
  • この商品を含むブログ (152件) を見る

メタプログラミングRuby 第2版

メタプログラミングRuby 第2版

新装版 リファクタリング―既存のコードを安全に改善する― (OBJECT TECHNOLOGY SERIES)

新装版 リファクタリング―既存のコードを安全に改善する― (OBJECT TECHNOLOGY SERIES)