Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| menus.each.with_index(1) do |menu, i| | ||
| numbered_menus = menus.each.with_index(1).to_h { |menu, i| [i, menu] } | ||
| numbered_menus.each do |i, menu| |
There was a problem hiding this comment.
この修正は特に必要ないように思いますが、どうでしょうか。
| order_number | ||
| menu = numbered_menus[order_number] | ||
| puts "#{menu[:name]}(#{menu[:price]}円)ですね。" | ||
| menu |
There was a problem hiding this comment.
メソッドのインターフェース(入力と出力の形)を変えることは今回はしないようにしてください。
バグ修正が目的なので、あまりおおげさな挙動変更は必要ありません。
There was a problem hiding this comment.
take_order の戻り値を元の番号に戻し、- 1 で調整する形に修正しました。
|
|
||
| puts 'bugカフェへようこそ！ご注文は？ 番号でどうぞ' | ||
| order1 = take_order(DRINKS) | ||
|
|
There was a problem hiding this comment.
今回の目的からすると不要な差分はないようにしてください。
yoshitsugu
left a comment
yoshitsugu
left a comment
There was a problem hiding this comment.
細かいと思われるかもしれませんが、不要な差分はコードレビューの雑音となりますので、たとえば不要行の削除など、コードフォーマットのリファクタリングを行う場合はバグ修正や機能追加などとは別のPull Request(PR)として、目的毎にPRをわける方法がよくとられます。
| puts "#{menus[order_number - 1][:name]}(#{menus[order_number - 1][:price]}円)ですね。" | ||
| order_number | ||
| end | ||
|
|
There was a problem hiding this comment.
ここも行削除の差分がでています。関係のない差分はできるだけないようにしましょう。
|
|
||
| puts 'bugカフェへようこそ！ご注文は？ 番号でどうぞ' | ||
| order1 = take_order(DRINKS) | ||
|
|
|
|
||
| puts 'フードメニューはいかがですか?' | ||
| order2 = take_order(FOODS) | ||
|
|
| @@ -1,34 +1,28 @@ | |||
| # frozen_string_literal: true | |||
|
|
|||
yoshitsugu
left a comment
yoshitsugu
left a comment
There was a problem hiding this comment.
よさそうです！
その上で、今後の事故防止という意味で、リファクタリングに近い修正になりますが、一点だけ考えてみてください。
発展的な話題ですが、そもそも order1 , order2 という変数名が今回の題材となった変数の取り違えのバグを誘発していそうですね。わかりやすいように変数名を変えてみてください。
|
@yoshitsugu |
3 participants
概要
cafe.rbに含まれていた3つのバグを修正しました。修正内容
1. メニュー番号と配列インデックスのズレ
表示上は1始まりの番号（
(1)コーヒー）だったのに対し、選択した番号をそのまま配列のインデックスとして使っていたため、1つズレたメニューが選ばれてしまっていました。→ 1始まりの番号をキーにしたハッシュ（
numbered_menus）を作ることで、ズレの原因になっていた-1の計算自体をなくしました。2. 合計金額計算時の配列の対応ミス
FOODS[order1] + DRINKS[order2]のように、ドリンクの注文番号でフード配列を、フードの注文番号でドリンク配列を参照してしまっていました。→
take_orderの戻り値を「選択されたメニューそのもの（ハッシュ）」に変更し、order1[:price] + order2[:price]のように直感的に計算できるようにしました。3. 価格が文字列になっていたことによる計算ミス
price: '300'のように価格が文字列で定義されていたため、+が数値の足し算ではなく文字列の連結として扱われ、合計金額が正しく計算されていませんでした（例:520460円のような表示）。→
price: 300のように数値として定義し直しました。