カレンダープログラムを追加 #2
Conversation
02.calendar/cal.rb
Outdated
|
|
||
| opt = OptionParser.new | ||
| opt.on('-y VAL') do |v| | ||
| v if (1..12).cover?(v.to_i) |
There was a problem hiding this comment.
そもそも範囲外の値が渡された場合の想定は今回しなくてよいですが、これだと範囲外の値のときに暗黙的に無視されてしまうので若干ユーザー側からすると不親切かなと思います。
abort や exit(1) をしてエラー表示 + エラー終了するほうがベターです。
https://docs.ruby-lang.org/ja/latest/method/Kernel/m/abort.html
https://docs.ruby-lang.org/ja/latest/method/Kernel/m/exit.html
| params[:m].to_i | ||
| end | ||
|
|
||
| class MyCal |
There was a problem hiding this comment.
クラス設計についてはオブジェクト指向のプラクティスでやるので、今回はあまりコメントしません。
02.calendar/cal.rb
Outdated
| end | ||
|
|
||
| def show | ||
| puts " #{@month}月 #{@year}" |
There was a problem hiding this comment.
全角スペースでのフォーマットは避けたほうがよいです。
String#center String#rjust String#ljust あたりが使えないか見てみてください。
https://docs.ruby-lang.org/ja/latest/method/String/i/center.html
https://docs.ruby-lang.org/ja/latest/method/String/i/rjust.html
https://docs.ruby-lang.org/ja/latest/method/String/i/ljust.html
02.calendar/cal.rb
Outdated
| rescue StandardError | ||
| break | ||
| end |
There was a problem hiding this comment.
これは複数のアンチパターン(よくないとされるパターン)を踏んでいるので、修正おねがいします。
- エラーを制御構文(if, whileなど)のように使うのはよくないです。エラーは
rescueされないとさらに上の呼び出し側に受け渡されますが、これを利用して、C言語などには存在するgotoのように一気にループを抜けたりするのに便利に使うこともできますが、そのようなコードが蔓延ってしまうと、どこからどこに行っているのかコード上での把握がかなり困難になってきます。デバッグなどの観点でよくないこととされます。 - エラーを暗黙的に処理するのはよくないです。特に今使っている
StandardErrorは大部分のエラーも補足してしまいます。そのため、想定外のエラーが発生してもそれを検知できません。
02.calendar/cal.rb
Outdated
| def show | ||
| puts " #{@month}月 #{@year}" | ||
| puts '日 月 火 水 木 金 土' | ||
| (1..31).each do |n| |
There was a problem hiding this comment.
そもそもDateクラスで末日を簡単に作れますので、調べてみてください。
また、Dateクラス自体をRangeとして扱えます。
first_day = Date.new(@year, @month,1)
last_day = Date.new(# どうなるか考えてみてください
(first_day..last_day).each do |date|
# この中ではdateが各日付をあらわす Dateオブジェクトになります。
end
02.calendar/cal.rb
Outdated
| youbi.times { print ' ' } if n == 1 | ||
|
|
||
| if d == Date.today | ||
| print "\e[30m\e[47m\e[5m#{d.day.to_s.rjust(2)}\e[0m" |
There was a problem hiding this comment.
今日の日付での反転に取り組まれていてよいですね 👍
ただ、文字色と背景色を反転させるためのエスケープコードは 7 です。
直接背景色や文字色を指定すると元々の色によっては「反転」にはなりませんので注意してください。
https://en.wikipedia.org/wiki/ANSI_escape_code#SGR_.28Select_Graphic_Rendition.29_parameters
02.calendar/cal.rb
Outdated
| end | ||
|
|
||
| if youbi == 6 | ||
| puts "\n" |
There was a problem hiding this comment.
puts は引数なしで改行を出力しますので、
putsのみで大丈夫です。
https://docs.ruby-lang.org/ja/latest/method/Kernel/m/puts.html
02.calendar/cal.rb
Outdated
| if (1..12).cover?(v.to_i) | ||
| v | ||
| else | ||
| abort "月の指定が正しくありません '#{v}'" |
There was a problem hiding this comment.
細かいですが、エラーメッセージのフォーマットは揃えておくほうが見やすいかと思います。
エラー: 正しく処理できない月です にするか、年のほうを月にあわせるほうがよいかと思います。
02.calendar/cal.rb
Outdated
| last_day = Date.new(@year, @month, -1) | ||
| (first_day..last_day).each.with_index(1) do |d, n| | ||
| youbi = d.wday | ||
| youbi.times { print ' ' } if n == 1 |
There was a problem hiding this comment.
わざわざループの中で条件つけてやる必要はないかと思うので、ループの前でやりましょう。
02.calendar/cal.rb
Outdated
| print d.day.to_s.rjust(2) | ||
| end | ||
|
|
||
| if youbi == 6 |
There was a problem hiding this comment.
d.saturday? が使えると思います。
https://docs.ruby-lang.org/ja/latest/method/Date/i/saturday=3f.html
There was a problem hiding this comment.
必須要件のスクリーンショットにある通り、 ./cal.rb で実行できるようにしてみてください！
参考: https://bootcamp.fjord.jp/articles/40
02.calendar/cal.rb
Outdated
| first_day = Date.new(@year, @month, 1) | ||
| last_day = Date.new(@year, @month, -1) | ||
|
|
||
| first_day.wday.times { print ' ' } |
There was a problem hiding this comment.
これはどちらでもいいのですが、String#* を使って
print ' ' * first_day.wdayと書くこともできますね。
https://docs.ruby-lang.org/ja/latest/method/String/i/=2a.html
No description provided.