調整さん問題をRubyで解く(改善編)

問題

前回の記事をご覧ください。

他の方の解答

回答いただきました。ありがとうございます。お二人ともRubyなのに、それぞれ違って面白いですね。

ツッコミ御礼

先日書いた自分の解答について色々ツッコミをもらいました。

  • リテラルを減らそう
  • Time/Dateのパーサを使おう
  • それがダメなら正規表現を使おう
    • テキスト処理に強い言語でsplintを使って頻繁に文字列ぶった切ってたら何かおかしいと思え、とのこと
  • パーシャルバインディングも使おう
    • 複数の戻り値を返す仕組みを使おうという話
  • 文字列の作成と出力を分けよう
  • ネストをやめよう

また、修正中のコードにもツッコミをもらいました。

  • returnを書くのはRubyらしくない
    • returnを書かなくていいように、戻り値にしたい値をメソッド内で最後に評価しよう

ありがとうございます。

自分のコード(改善編)

というわけで、修正しました。

#!/usr/bin/ruby
# coding: utf-8
require 'date'

MIN = 60

def get_day(day)
  Date.strptime(Time.now.year.to_s << "/" << day, "%Y/%m/%d")
end

def parse(input)
  if /^([01]?[0-2]\/[0-3]?\d)-([01]?[0-2]\/[0-3]?\d)\s([0-2]?\d):([0-5]?\d)-([0-2]?\d):([0-5]?\d)\s(\d)/ =~ input.join(" ") then
    return $1, $2, $3.to_i, $4.to_i, $5.to_i, $6.to_i, $7.to_f
  else
    puts "mm/dd-mm/dd H:M-H:M h形式で日付を入力"
    exit
  end
end

def create_boxes(bh, bm, eh, em, box)
  t = (((eh * MIN + em) - (bh * MIN + bm)) / (box * MIN)).to_i
  ((0...t).to_a).map{|n|
      t_eh = (((bh * MIN + bm) + box.to_f * MIN) / MIN).to_i
      t_em = (((bh * MIN + bm) + box.to_f * MIN) % MIN).to_i
      t_bh, t_bm = bh, bm
      bh, bm = t_eh, t_em
      sprintf("%02d:%02d-%02d:%02d", t_bh, t_bm, t_eh, t_em)
  }
end

def format(d)
    wdays = ["", "", "", "", "", "", ""]
    d.strftime("%m/%d(#{wdays[d.wday]})")
end
  
def create_lines(day, times)
  times.map {|time|
    (format(day) << " " << time)
  }
end

bd, ed, bh, bm, eh, em, box = parse(ARGV)
days = (get_day(bd) ... get_day(ed)).to_a
boxes = create_boxes(bh, bm, eh, em, box)

days.map {|day|
  create_lines(day, boxes)
}.flatten.each { |line| puts line}

E_Mattsanさんを見習って、正規表現を使いました。

さらなる改善

メソッドに切り出しているので目立たないけど、create_linesメソッドの呼び出し箇所でmapがネストしています。日程の配列に対して、時間枠の配列をあてがっているので、どうしてもこうなってしまう。

これをフラットにしたいですが、どうしたらいいか考え中です。