添削した
#ruby -Ks filename = "access.log" ids = Hash::new file = open(filename,"r"){|file| file.each{|line| finded_id = line.scan(/id=(.*)&type/) rss = /&type=rss/=~line html = /&type=html/=~line unless ids[finded_id[0] ] ids[finded_id[0] ]= Hash::new ids[finded_id[0] ]["name"] = finded_id[0].to_s ids[finded_id[0] ]["count"] = 0 ids[finded_id[0] ]["rss"] = 0 ids[finded_id[0] ]["html"] = 0 end ids[finded_id[0] ]["count"] += 1 if finded_id[0] ids[finded_id[0] ]["rss"]+= 1 if html ids[finded_id[0] ]["html"]+= 1 if rss } } result_arr = ids.values sorted = result_arr.sort{|a, b| if a["count"].to_i > b["count"].to_i -1 else 1 end } sorted.each{|name| p name }もっとスマートな書き方があると思うので添削よろ。
ということで添削した。
解説なしバージョン
filename = "access.log" user_data = Hash.new File.open(filename,"r").each{|line| next unless id = line.scan(/id=(.*)&type/).flatten[0] user_data[id] ||= { :count => 0, :rss => 0, :html => 0 } user_data[id][:count] += 1 user_data[id][:rss] += 1 if /&type=html/=~line user_data[id][:html] += 1 if /&type=rss/=~line } user_data.to_a.sort{|a,b| b[1][:count] <=> a[1][:count]}.each{|user| p user}
ちょっと解説があるバージョン
filename = "access.log" user_data = Hash.new # FileオブジェクトはIOオブジェクトを継承しているので # 直にeachできる。 File.open(filename,"r").each{|line| # finded_id = line.scan(/id=(.*)&type/) # 戻り値は[[$1], [$1], ...]だからfinded_id[0]は$1じゃなく[$1] next unless id = line.scan(/id=(.*)&type/).flatten[0] # user_data[finded_id[0] ]= Hash::new # user_data[finded_id[0] ]["name"] = finded_id[0].to_s # user_data[finded_id[0] ]["count"] = 0 # user_data[finded_id[0] ]["rss"] = 0 # user_data[finded_id[0] ]["html"] = 0 # Javaっぽい。DRYじゃない user_data[id] ||= { :count => 0, :rss => 0, :html => 0 } user_data[id][:count] += 1 # if id # id==nilならブロックの頭でnextされている user_data[id][:rss] += 1 if /&type=html/=~line user_data[id][:html] += 1 if /&type=rss/=~line } # result_arr = user_data.values # sorted = result_arr.sort{|a, b| # if a["count"].to_i > b["count"].to_i # -1 # else # 1 # end # } # sorted.each{|name| # p name # } # 冗長 # とはいえこれも綺麗ではなさげ user_data.to_a.sort{|a,b| b[1][:count] <=> a[1][:count]}.each{|user| p user}
添削しといてTypoがあったりするのもあれなので、一応検証用のデータを3秒くらいで作成して、「寝起きでリファクタして1発で動いたらツンデレ」とか意味不明なことを呟きながら
ruby kikker.rb
で動いたのでよしとする。
しかし、
user_data[id][:count] += 1 user_data[id][:rss] += 1 if /&type=html/=~line user_data[id][:html] += 1 if /&type=rss/=~line
あたりがどうもDRYじゃない。