Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

20240209【Ruby】プログラムの修正 #1

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

taichihub
Copy link
Owner

@taichihub taichihub commented Feb 9, 2024

issue

プログラムの修正

修正箇所

  • フード,ドリンクのインデックス番号指定ミスを修正
  • フード,ドリンクの定数が逆になっていたのを修正
  • 合計金額が文字列同士の足し算になっていたのでintegerに変換して足し算されるようにした

実行結果

image

@taichihub taichihub self-assigned this Feb 9, 2024
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.

コメントしました!

Gemfile Outdated

ruby "3.2.1"

gem 'debug', '~> 1.9', '>= 1.9.1'

Choose a reason for hiding this comment

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

debugはBundlerを使わなくてもRuby標準で使用できます(dateライブラリなどと同じ)。
もし何か理由があってBundlerを使っているなら理由を教えてください〜

Copy link
Owner Author

Choose a reason for hiding this comment

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

@JunichiIto
特に理由は無かったので削除しようと思います💦

cafe.rb Outdated
@@ -20,7 +20,7 @@ def take_order(menus)
end
print '>'
order_number = gets.to_i
puts "#{menus[order_number][:name]}(#{menus[order_number][:price]}円)ですね。"
puts "#{menus[order_number - 1][:name]}(#{menus[order_number - 1][:price]}円)ですね。"

Choose a reason for hiding this comment

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

order_numberを定義する時点で1減らした方が変更量が少なく済みそうです

cafe.rb Outdated
@@ -30,5 +30,5 @@ def take_order(menus)
puts 'フードメニューはいかがですか?'
order2 = take_order(FOODS)

total = FOODS[order1][:price] + DRINKS[order2][:price]
total = DRINKS[order1 - 1][:price].to_i + FOODS[order2 - 1][:price].to_i

Choose a reason for hiding this comment

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

たしかにこの修正でも正しく動きますが、そもそも価格が文字列で定義されているのが妥当かどうか考えてみてください。
それと、変数名は連番だとうっかりミスでバグを埋め込みやすくなるので、もっとわかりやすい変数名も考えてみてほしいです〜。

@taichihub
Copy link
Owner Author

@JunichiIto
レビューありがとうございます!修正対応進めていきます🙇‍♂️

@taichihub
Copy link
Owner Author

@JunichiIto
レビュー修正対応しました!

#1 (comment)

特に理由は無かったのでGemfile, Gemfile.lockを削除しました。

#1 (comment)

order_numberの時点で - 1 をし、他農部分で書いていた - 1 の記述をなくしました。

#1 (comment)

文字列で定義されていた価格を定義段階で数値に変更し、.to_iを削除しました

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点コメントしましたが、変数名を直せば終わりなので、課題としてはこれでOKとしますー!

order_number
end

puts 'bugカフェへようこそ!ご注文は? 番号でどうぞ'
order1 = take_order(DRINKS)
drink = take_order(DRINKS)

Choose a reason for hiding this comment

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

これだと飲み物そのものが変数に格納されてそうな印象を受けます。
あくまで注文の情報なので、 drink_order (飲み物の注文)みたいな名前の方が良いと思います。

メソッド名と変数名の対応関係、という観点でも命名を確認してみるとよいかもしれません。
https://bootcamp.fjord.jp/pages/readable-variable-name

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.

2 participants