プレゼントの抽選をするスクリプトを添削してみた

F's Garage:プレゼントの抽選をするプログラムをrubyで書いてみた。で書かれているコードが面白そうだったので添削してみました。

配列のシャッフル

class Array
  def shuffle
    arr = dup
    collect{arr.slice!(rand(arr.length))}
  end
end
F's Garage:プレゼントの抽選をするプログラムをrubyで書いてみた。

4行目の動きがよくわからないけど、ランダムに並べ替えたいというだけならsort_byとrandを使うのが定石で、以下のように置き換えられます*1

class Array
  def shuffle
    sort_by{rand}
  end
end

CSV列を配列の配列に変換

cr = CSV.open(filename , 'r')

arr = Array.new
cr.each{|line|
  arr.push(line)
}
F's Garage:プレゼントの抽選をするプログラムをrubyで書いてみた。

この部分は、CSV.openがEnumerableをincludeしたオブジェクトを返すので、Enumerable#to_aが使用できます。
また、これは使い捨てコードなので実害は無いかもしれませんが、crがクローズされてないのが気になるので、それを踏まえると以下のように置き換えられます*2

cr = CSV.open(filename , 'r')
arr = cr.to_a
cr.close

また、もっと単純に、CSV#readというメソッドがCSVを配列の配列の形にしてくれるので、以下のように置き換えることもできます。

arr = CSV.read(filename)

Rubyは便利な組み込みメソッドが多いので、まずはリファレンスを当たってみるのが楽をする近道です。

抽選

random_arr.each{|line|
  price = line[5].to_i
  if total + price < max then
    total= total + price
    tousen.push(line)
  end
}
F's Garage:プレゼントの抽選をするプログラムをrubyで書いてみた。

ここには特に口を出すところはないと思います。強いてあげれば「total= total + price」が「total += price」と書けることと、複数行のブロックに{}はあまり使われない、ということくらいでしょうか。

自分ならこう組む

同じような仕様で自分が組むならこんな感じかなと思います。
配列を作った後にCSVに書き込んでいるようですが、特に別にする必要もないんじゃないかと思って一緒にしてあります。
あとmaxとtotalの宣言が離れてるのは、定数と内部処理用の変数という意味が違うものじゃないかなという個人的な趣味です。

require "CSV"

filename = ARGV[0] || "present_original.csv"
max = 1000000

applicants = CSV.read(filename).sort_by{rand} # 1.8.7以上ならArray#shuffle

total = 0
CSV.open("tousen.csv" , "w") do |writer|
  applicants.each do |applicant|
    price = applicant[5].to_i
    next if total+price > max
    writer << applicant
    total += price
  end
end
printf "chusen finish total price is %d \n" , total

*1:Ruby1.8.7以上ならArray#shuffleは組み込みです

*2:こういう場合はブロックで……と思ったら、ブロックにすると列ごとの配列になってしまうので