Skip to content

Conversation

@blacktoad30
Copy link
Owner

  • cafe.rb: Change ":price" values to Integer objects, and sum the prices of the selected menus.
    (take_order): Choose the correct menu.

* cafe.rb: Change ":price" values to Integer objects, and sum the
prices of the selected menus.
(take_order): Choose the correct menu.
Copy link

@JunichiIto JunichiIto left a comment

Choose a reason for hiding this comment

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

1点コメントしました〜。

cafe.rb Outdated
Comment on lines 21 to 24
until (order_number ||= -1).between?(0, menus.size - 1)
print '>'
order_number = gets.to_i - 1
end

Choose a reason for hiding this comment

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

Suggested change
until (order_number ||= -1).between?(0, menus.size - 1)
print '>'
order_number = gets.to_i - 1
end
begin
print '>'
order_number = gets.to_i - 1
end while menus[order_number].nil?

こんな感じにした方がシンプルかつコードの意図が明確になるかな?と思いますがいかがでしょう?

Copy link
Owner Author

Choose a reason for hiding this comment

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

レビューしていただき、ありがとうございます。
begin...end whileのほうが読みやすいと感じます。

修正して実行してみたところ、Rubyの配列の仕様により、order_number- menus.size + 1以上0以下の整数でも動作しました。これは許容しても良いのでしょうか?
自分としては、提示されたメニューの番号を入力すべきだと考えていますが、begin...end until (または while !) order_number.between?(0, menus.size - 1)では意図が伝わりにくいでしょうか?

Choose a reason for hiding this comment

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

なるほど、たしかにwhile menus[order_number].nil?だと元のコードと挙動が異なりますね。
end until (または while !) order_number.between?(0, menus.size - 1) みたいな感じで良いと思います!

ちなみにGitHubのコメント通知はほかの通知メールに埋もれて気づけないときがあるので、今後は念のためFBC側のコメント欄でも「PRで質問してます」みたいな連絡をもらえると助かります〜 🙏

Copy link
Owner Author

Choose a reason for hiding this comment

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

なるほど、たしかにwhile menus[order_number].nil?だと元のコードと挙動が異なりますね。 end until (または while !) order_number.between?(0, menus.size - 1) みたいな感じで良いと思います!

ありがとうございます!
コードを修正した後、pushします。

ちなみにGitHubのコメント通知はほかの通知メールに埋もれて気づけないときがあるので、今後は念のためFBC側のコメント欄でも「PRで質問してます」みたいな連絡をもらえると助かります〜 🙏

承知しました。コメント欄のほうにも記入するようにします。

* cafe.rb (take_order): Change until statement to begin statement with
until modifier.
@blacktoad30
Copy link
Owner Author

@JunichiIto コードを修正しました!
ご確認のほどよろしくお願いいたします。

Copy link

@JunichiIto JunichiIto left a comment

Choose a reason for hiding this comment

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

確認しました!OKです〜🙆‍♂️

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