ボウリングスコア計算プログラムの追加 #3
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
yuuu
reviewed
Nov 8, 2023
03.bowling/bowling.rb
Outdated
| end | ||
|
|
||
| frames = [] | ||
| (1..10).each do |i| |
There was a problem hiding this comment.
ここは each よりも map を使うとコードがスッキリしますね。
frames = (1..10).map do |i|
# frameに挿入したい値を戻り値にする
endThere was a problem hiding this comment.
引数 i だと何を指しているのか分かりづらいですし、今回の場合 i は未使用となっていますので、引数を省略しましょう。
03.bowling/bowling.rb
Outdated
| # 10フレーム目 | ||
| if i == 9 | ||
| # 単に足すだけ | ||
| total_point += frame.sum |
There was a problem hiding this comment.
この加算式は全ての分岐に存在するので、分岐の前後に移動させることでコードの重複をなくせるはずです
03.bowling/bowling.rb
Outdated
| elsif i == 8 | ||
| total_point += frame.sum | ||
| # ストライク | ||
| if frame[0] == 10 |
There was a problem hiding this comment.
フレームを判定してからストライクorスペアを判定していることで、ストライク・スペアの計算処理が2回記述されてしまっています。
先にストライクorスペアを判定して、それからフレームを判定すれば、フレームによる処理の違いのみを分岐させられるので、コードの重複を減らせるはずです。
03.bowling/bowling.rb
Outdated
| total_point += frame.sum | ||
| # ストライク | ||
| if frame[0] == 10 | ||
| total_point += frames[i + 1][0] |
There was a problem hiding this comment.
この2行は以下のように書くと1行にまとめられますね。
※他の指摘の修正によりまとめられなくなるかもしれませんが、参考まで。
total_point += frames[i + 1][0..1].sum| @@ -0,0 +1,9 @@ | |||
| ./bowling.rb 6,3,9,0,0,3,8,2,7,3,X,9,1,8,0,X,6,4,5 #-> 139 | |||
03.bowling/bowling.rb
Outdated
| end | ||
|
|
||
| total_point = 0 | ||
| frames.each_with_index do |frame, i| |
There was a problem hiding this comment.
可能であればここでも sum を使うと、total_pointを自分で加算する必要がなくなりコードがスッキリするのでおすすめです。
total_point = frames.each_with_index.sum do |frame, i|
# 各フレームのスコアをブロックの戻り値にする
end
yuuu
reviewed
Nov 9, 2023
03.bowling/bowling.rb
Outdated
| end | ||
| # スペア | ||
| elsif frame.sum == 10 | ||
| total_point += frames[i + 1][0] |
There was a problem hiding this comment.
最終フレームがスペアの場合に存在しないフレームを参照してしまう、という懸念はないでしょうか？
03.bowling/bowling.rb
Outdated
| end | ||
|
|
||
| total_point = 0 | ||
| frames.each_with_index do |frame, i| |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.