るびー めも

Ruby の学習メモを記す

とある Ruby ソースのリファクタリングについて

前回の記事の続きです。

とあるプルリクエストで採用されたコードは、無駄がいっぱいあったのと
メンテナンス性が悪そうだったので、リファクタリングしてみました。

まず、以下の問題がありました。
10000 回ほど、連続して実行させると結果が少し偏る

$ ./roulette.rb M2
ふじい: 2937
すがわら: 2023
おくつ: 2662
なかじま: 2378
$ ./roulette.rb M2
ふじい: 3026
なかじま: 2366
すがわら: 2035
おくつ: 2573
$ ./roulette.rb M2
ふじい: 3089
すがわら: 2074
なかじま: 2183
おくつ: 2654

上記は、rand(10) を rand(100)にすることで結果が(少し)均等になりました。

$ ./roulette.rb M2
ふじい: 2521
すがわら: 2379
なかじま: 2526
おくつ: 2574
$ ./roulette.rb M2
おくつ: 2527
ふじい: 2545
なかじま: 2396
すがわら: 2532
$ ./roulette.rb M2
すがわら: 2398
おくつ: 2474
なかじま: 2470
ふじい: 2658

次に、使用者の目線で考えると Roulette クラスは複数のインスタンス
生成したりすることはなさそうなので、new する必要もないだろうということで
start/speak は、クラスメソッド化しました。

おかげさまで、Roulette.new(arg).start ではなくて
Roulette.start(arg) という風に自然な感じの使い方が出来そうです。

最後に、やたら出てくるコマンドのパスがメンテナンス性を下げていたため
定数に変更し、メンテナンス性を高めました。

以下は、再度プルリクエスト中のソースコードです。

#!/usr/bin/env ruby
# -*- coding: utf-8 -*-

SAY    = "/usr/bin/say"
PLAYER = "/usr/bin/afplay"
BGM    = "./gaki.mp3"

group  = { "M2" => %w(おくつ すがわら なかじま ふじい),
           "M1" => %w(たけみち こまつ ふるだて),
           "B4" => %w(おくとも かとう いのうえ あべ なかむら ひらが まいた かどわき ほし やぎぬま) }

class Roulette 
  def self.start members
    @result = Hash.new
    @members = members
    @members.each do | member |
      @result[member] = rand(100) + 1
    end
    @result = @result.sort{|a, b| b[1] <=> a[1]}
    if File.exist?("#{SAY}")
      speak
    end
    return @result
  end
  private
  def self.speak 
    result = @result.dup
    result.each_with_index do | elem, index |
      3.times { print "."; sleep(0.5) }
      puts message = "#{elem.join(", ")}点。"
      `#{SAY} #{message}`
      if index == result.length - 1
        threads = []
        puts message = "#{elem[0]}" + "ーーー。アウトーーー。"
        threads.push(Thread.new{`#{PLAYER} #{BGM}`})
        sleep(4)
        threads.push(Thread.new{`#{SAY} #{message}`})
        threads.each{|thread| thread.join}
      end
    end
  end
end

attr = ARGV[0] 
if group.include?(attr)
  Roulette.start(group[attr])
else
  Roulette.start($*)
end

とりあえず、現状これがせいいっぱいなんですが
もっとリファクタリングできるよ!ってな人はコメントお願いします。