Skip to content

Conversation

@1mori
Copy link
Owner

@1mori 1mori commented May 10, 2024

No description provided.

else
print "\n"
end
end No newline at end of file
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

細かいですが、末尾改行しましょうー

require 'optparse'

# 今日の日付を取得する
today_date = Date.today
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

細かいですが、=の後ろは半角空白1つにしましょう


# 今日の日付を取得する
today_date = Date.today
date_params = {year: today_date.year, month: today_date.month, day: today_date.day}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

細かいですが、{}の前後空白あけたいですねー

require 'date'
require 'optparse'

# 今日の日付を取得する
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

説明的なコメントは原則なくしたいですね。
参考
https://qiita.com/jnchito/items/f0d90af4ed44b7484103

opt.on('-y [VAL]') {|v| v.to_i}
opt.on('-m [VAL]') {|v| v.to_i}

opt.parse!(ARGV, into: cmd_params)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

into知らなかったです。便利ですねー 👍

else
date_params[:month] = cmd_params[:m]
end
end
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

後述してますが、今日の判定を見直すとsame_ymなくせそうですね。するとこの辺のロジックもすっきりしそうだな、と思います。

#色の反転
def reverse_cmpcolor(text)
"\e[7m#{text}\e[0m"
end
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

他のところメソッド化してないのにここだけ急にメソッドなことに違和感を感じます。なぜここだけメソッドなのですか?


# カレンダー部分の出力
# 余白部分の空白を予め出力しておく
dow_num = first_date.wday
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dow_numがよくわからない命名だなと思いました。dowってなんですか? date of week? 一般的な略称なんでしょうか? あまり一般的でないなら変に省略すると可読性が下がるかな、と思います。

# カレンダー部分の出力
# 余白部分の空白を予め出力しておく
dow_num = first_date.wday
dow_num.times { print " " }
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

必須対応ではないですが、文字列で掛け算をするとすっきりかけそうです。

dow_num.times { print " " }

# 日にちの出力
(first_date.day..last_date.day).each do |num|
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ブロック変数にもっと具体的な名前をつけましょう。numではなんのnumなのかわからないですよね。

if same_ym == true && num == date_params[:day]
print reverse_cmpcolor(num.to_s.rjust(2))
(first_date..last_date).each do |current_date|
if current_date.year == date_params[:year] && current_date.month == date_params[:month] && current_date.day == date_params[:day]
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

today_dateせっかくあるのでうまく活用しましょうー。Date型同士で比較するとシンプルですね

print current_date.day.to_s.rjust(2)
end
if dow_num == 6
if day_of_week_index == 6 # 曜日リセット
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://docs.ruby-lang.org/ja/latest/method/Date/i/saturday=3f.html というのが全曜日分あったりします。このへんうまく使うとday_of_week_indexは無くせるのではないかな、と思いました。いかがでしょうか。


# 日にちの出力
(first_date..last_date).each do |current_date|
if current_date.year == today_date.year && current_date.month == today_date.month && current_date.day == today_date.day
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Date型同士っていうのはこういうことじゃないです。これだと年(Integer)、月(Integer), 日(Integer)それぞれで比較ですよね。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants