はじめに
この記事は「ボウリングのスコアを計算する
クラスを作る」という問題を先輩から出題していただき、それを自力で解き、先輩のレビューをいただき振り返る過程を纏めた記事です。
備忘録・振り返り的な要素が強い為、フランクな書き方をしておりますが、大目に見ていただければと思います。軽い読み物だと思って読んでください。
また解き方に稚拙な箇所もあるかと思いますがご容赦ください。指摘等は大歓迎です。励みになります。
登場人物
-
私
- 社会人2年目PG。Rubyでコーディングし始めて1年とちょっと。うっかりポンコツ。最近Reactを書き始めた。前回の問題以来、
each_with_object
とgroup_by
と仲良くなった。
- 社会人2年目PG。Rubyでコーディングし始めて1年とちょっと。うっかりポンコツ。最近Reactを書き始めた。前回の問題以来、
-
コウイカちゃん
- 私の心の中に住んでいるイカ。 腹黒い。イカだけに。助言をくれたりくれなかったりする。
-
ホタルイカ先輩
- コウイカちゃんの先輩。沖漬けにされるのが好き。コーディングが得意。
出題された問題
今回ホタルイカ先輩から出題された問題は以下です。
以下の要件に従って問題を解いていきます。
ボウリングスコア計算
問題
- ボウリングのスコアを計算
- 投球を完了した状態で最終的なスコアを算出する。
入力例
- 10 フレーム分の投球を配列とする
- 各フレームも配列とする
[
[1, 1], # 1投目
[2, 2], # 2投目
[3, 3], # 3投目
[4, 4], # 4投目
[1, 1], # 5投目
[2, 2], # 6投目
[3, 3], # 7投目
[4, 4], # 8投目
[1, 1], # 9投目
[2, 2] # 10投目
]
- ストライクがある場合、フレームの配列は 1 つの要素とする
- 10 フレーム目は、スペアまたはパンチアウト(全てストライク)の場合、3 つの要素とする
- ガターの場合、0 とする
[
[1, 1], # 1投目
[0, 2], # 2投目
[3, 0], # 3投目
[4, 4], # 4投目
[5, 5], # 5投目
[1, 1], # 6投目
[10], # 7投目
[4, 4], # 8投目
[1, 1], # 9投目
[5, 5, 5] # 10投目
]
要求仕様
- 最終スコアを数値(整数)で返す
- 入力値が整数ではない、ボウリングのルール上計算出来ない値の場合、例外を返す(例外はカスタム例外を作成する)
- カレントフレームスコアシステムは適用しない(これまで通りの計算方法)
フェーズ1 自分で考える
イメージを膨らませる
私「コウイカちゃんきいて」
コウイカ「どうしたの私ちゃん」
私「すごい重要なことに気付いたの」
私「私ボウリングのスコアの計算方法知らない……(片手で収まる程度にしかボウリングをやったことがない)」
コウイカ「まじ?」
コウイカ「これだから隠キャは……。」
私「きっっっっっっっっっつ」
コウイカ「イカ界ではボウリングのスコア計算なんて呼吸の仕方と同じくらい当たり前のことだわ。」
私「そっかあ(「ボウリング スコア 計算」 で検索)」
ボウリングのルール
- スペアを投げた場合
- スペアの次の1投で倒した本数を加算する
- ストライクを投げた場合
- ストライク後に投げた2投で倒した本数を加算する
私「な、なるほど?」
私「じゃあ ストライク
・ストライク
・ストライク
って投げたとき、最初のストライクのスコアは30
になるってことだね?」
コウイカ「そうよ。」
私「なるほど……じゃあストライクはストライクが連続した場合
と連続しない場合
を考慮しなくちゃいけないんだね。」
私「あ。最終フレームで三回投げられる時ってあるよね。あれってどんなときなの?」
コウイカ「最初の1投がストライク、もしくは1投2投がスペアだった場合、3投投げることができるわよ。」
私「なるほど〜!スコア計算はそんな感じね。一旦例外を起こさなきゃいけない条件をまとめてみようかな。]
- 引数に渡された配列に文字列が含まれている場合
- フレーム数が
10
を超えている - 1フレームの配列の合計が
10
にならない場合 -
1〜10
以外の数字が含まれている場合 - スコアを格納している配列の長さが不適当な場合
私「こんな感じかな!これでコーディング進めてみよ!」
イメージを元にコーディングしていく
私「とりあえずざっくりと形を作ってみよう。」
class Bowling
def initialize(score)
@score = score
end
def result
total_score = 0
@score.each do |points_arr|
# スコアを計算する
total_score += calculate_score
end
total_score
end
end
私「めちゃくちゃシンプルに書くとこうよね」
コウイカ「極論はね。ここから例外のことだったり計算処理だったりを付け足していけばきっといい感じにできると思うわ。」
私「オッケー。まず初めにボウリングのルール上計算出来ない
場合に例外を発生させよう。例外も一緒に作ってしまおうかな。」
class Bowling
def initialize(score)
@score = score
end
def result
raise UncalculatableError unless valid_score?
total_score = 0
@score.each do |points_arr|
# スコアを計算する
total_score += calculate_score
end
total_score
end
private
def valid_score?
@score.count == 10 && @score.flatten.all? { |point| (0..10).include?(point) }
end
end
class UncalculatableError < StandardError; end
私「以下の場合は計算をする必要がないから早期リターンする。」
- フレーム数が10じゃない場合
- 配列に
1〜10
以外の値が含まれている場合
私「で、10フレーム目だけ色々しなきゃいけないから、フレーム数をカウントしたいんだよね。each.with_index
使うかな。」
class Bowling
def initialize(score)
@score = score
end
def result
raise UncalculatableError unless valid_score?
total_score = 0
@score.each.with_index(1) do |points_arr, frame_num|
# スコアを計算する
total_score += calculate_score
end
total_score
end
# 略
私「う〜〜ん。あと1フレームの配列の合計が10にならない場合
とスコアを格納している配列の長さが不適当な場合
はこの中で例外出さなきゃだから……とりあえず1フレーム目以外で計算しちゃダメなやつを弾こう。」
class Bowling
# 略
def result
raise UncalculatableError unless valid_score?
total_score = 0
@score.each.with_index(1) do |points_arr, frame_num|
raise UncalculatableError if invalid_points?(points_arr, frame_num)
return total_score += calculate_final_throw_score(points_arr) if frame_num == 10
total_score += calculate_score(points_arr, frame_num)
end
end
# 略
private
def invalid_points?(points_arr, frame_num)
points_arr.empty? || frame_num < 10 && points_arr.count > 2
end
end
私「配列が空の場合、それから10フレーム目以外で配列の長さが2じゃなかったら例外を起こす!!10フレーム以外で2回投げてないのはおかしいからね。」
コウイカ「なるほどね。ってことは10フレーム目の時の配列の長さのことも考慮しないといけないわね?
」
私「そだねい〜!じゃあ次計算ロジックだ!とりあえず、10回目以外の時の計算ロジックを考えよう!」
計算ロジックを考えていく
class Bowling
# 略
def result
raise UncalculatableError unless valid_score?
total_score = 0
@score.each.with_index(1) do |points_arr, frame_num|
raise UncalculatableError if invalid_points?(points_arr, frame_num)
total_score += calculate_score(points_arr, frame_num)
end
end
# 略
private
def calculate_score(points_arr, frame_num)
total = points_arr.sum
total += calculate_strike_score(points_arr, frame_num) || 0
total += calculate_spare_score(points_arr, frame_num) || 0
total
end
def calculate_strike_score(points_arr, frame_num)
return @score[frame_num][0] + @score[frame_num + 1][0] if continuously_strike?(points_arr, frame_num)
@score[frame_num][0] + @score[frame_num][1] if strike?(points_arr)
end
def calculate_spare_score(points_arr, frame_num)
@score[frame_num][0] if spare?(points_arr)
end
def continuously_strike?(points_arr, frame_num)
@score[frame_num].count == 1 && strike?(points_arr)
end
def strike?(points_arr)
points_arr.count == 1 && points_arr[0] == 10
end
def spare?(points_arr)
points_arr.count == 2 && points_arr.sum == 10
end
end
私「最初にも言ったけど、今回は計算が4パターンある。」
- ストライクが連続する場合(例:3投連続でストライク)
- ストライクの場合
- スペアの場合
- ストライクでもスペアでもない場合
私「本当は各calculate
メソッドを呼ぶところで後置if
したいんだけど(イメージは以下)、RuboCop
に怒られるからメソッドの中で上記4パターンを判定する。条件に一致しない場合はnil
が返っちゃうからnil
が返る場合は0を足すようにするよ。」
def calculate_score(points_arr, frame_num)
total = points_arr.sum
total += @score[frame_num][0] + @score[frame_num + 1][0] if continuously_strike?(points_arr, frame_num)
total += @score[frame_num][0] + @score[frame_num][1] if strike?(points_arr)
total += @score[frame_num][0] if spare?(points_arr)
total
# これはABCSizeで怒られる
end
私「基本は『倒した本数+ボーナス点』でそのフレームのスコアを計算する。」
コウ「上のそれぞれの場合に合わせてボーナス点が変動する感じだね。」
私「そうだよ〜!…………うーん、ボーナス点を計算する時、フレーム数(1始まり)と配列の要素番号(0始まり)がズレてるから配列から何を取ってるか直感的に分かりにくいかなぁ……。でもif文の条件式はフレーム数ベースの方が分かりやすいんだよね……そっち優先しよう。」
私「あとは10投目を計算するだけ!よし!!」
class Bowling
# 略
def result
raise UncalculatableError unless valid_score?
total_score = 0
@score.each.with_index(1) do |points_arr, frame_num|
raise UncalculatableError if invalid_points?(points_arr, frame_num)
return points_arr.sum if frame_num == 10
total_score += calculate_score(points_arr, frame_num)
end
end
私「よしよし!これでいいんじゃないかな!!おやつ食べよ!」
・
・
・
・
・
コウイカ「何か忘れてるんじゃない?」
私「ほ?」
コウイカ「自分で言ってたわよ」
私「????」
私「アッ!?!?!?!3投目のバリデーション!!!」
コウイカ「(せいかいのおと)」
私「ええっと……3投目でありえそうなことは……」
- 1投目がストライクだが3回投げていない
- 1投目2投目でスペアになっているが3回投げていない
私「こんな感じかな?これでそれ用のメソッドを定義して……と。」
class Bowling
# 略
def result
raise UncalculatableError unless valid_score?
total_score = 0
@score.each.with_index(1) do |points_arr, frame_num|
raise UncalculatableError if invalid_points?(points_arr, frame_num)
return total_score += calculate_final_throw_score(points_arr) if frame_num == 10
total_score += calculate_score(points_arr, frame_num)
end
end
private
def calculate_final_throw_score(points_arr)
raise UncalculatableError if points_arr[0] == 10 && points_arr.count < 2
raise UncalculatableError if (points_arr[0] + points_arr[1] == 10) && points_arr.count < 3
points_arr.sum
end
end
私「できた!!」
私「全部ストライクの時、全部スペアの時、ちゃんと想定した計算ができてるし、オッケーだな!」
私「RuboCop
しんどかったな……後置ifを1メソッドの中に3つ書くとそれだけでABCSize
って怒られるんだもんな……。RuboCop
のためだけにメソッドになりました!ちょっと不細工なメソッドになりました!みたいなメソッドが多すぎる……。どうするのがいいんだろうな……。」
実際に完成したソース
class Bowling
def initialize(score)
@score = score
end
def result
raise UncalculatableError unless valid_score?
total_score = 0
@score.each.with_index(1) do |points_arr, frame_num|
raise UncalculatableError if invalid_points?(points_arr, frame_num)
return total_score += calculate_final_throw_score(points_arr) if frame_num == 10
total_score += calculate_score(points_arr, frame_num)
end
end
private
def valid_score?
@score.count == 10 && @score.flatten.all? { |point| (0..10).include?(point) }
end
def invalid_points?(points_arr, frame_num)
points_arr.empty? || frame_num < 10 && points_arr.count > 2
end
def strike?(points_arr)
points_arr.count == 1 && points_arr[0] == 10
end
def spare?(points_arr)
points_arr.count == 2 && points_arr.sum == 10
end
def continuously_strike?(points_arr, frame_num)
@score[frame_num].count == 1 && strike?(points_arr)
end
def calculate_score(points_arr, frame_num)
total = points_arr.sum
total += calculate_strike_score(points_arr, frame_num) || 0
total += calculate_spare_score(points_arr, frame_num) || 0
total
end
def calculate_strike_score(points_arr, frame_num)
return @score[frame_num][0] + @score[frame_num + 1][0] if continuously_strike?(points_arr, frame_num)
@score[frame_num][0] + @score[frame_num][1] if strike?(points_arr)
end
def calculate_spare_score(points_arr, frame_num)
@score[frame_num][0] if spare?(points_arr)
end
def calculate_final_throw_score(points_arr)
raise UncalculatableError if points_arr[0] == 10 && points_arr.count < 2
raise UncalculatableError if (points_arr[0] + points_arr[1] == 10) && points_arr.count < 3
points_arr.sum
end
end
class UncalculatableError < StandardError; end
フェーズ1まとめ
- 場合分けは一通り最初に考えておく(今回はテストファーストでやったので、いつもに比べてコーディングの手が止まることがなかった)
- 大枠から作っていくとイメージをつけやすい(今回は最低限の骨組みだけ作って、ちょっとずつ肉付けしていった)
-
RuboCop
のABCSize
を回避するためにも、同じようなジャンルの処理はメソッド単位でパーツ化する(今回は場合分けした単位でメソッド化していった)
フェーズ2 レビューをいただく
私「ホタルイカ先輩、レビューお願いします!!!」
ホタ「よし。」
ホタ「まず最初に、俺が用意したテスト6つ落ちました
」
私「えっっっっっっっっっっ」
ホタ「計算ロジックの考え方は良かったけど、バリデーション系で落ちてた。考慮漏れがあったってことだな」
私「(絶望)」
※漏れていた考慮は以下
- 引数の型が不正
- 投球に小数が含まれている
- フレーム毎の投球数が不足している
- 10フレームの投球数がミスで3投
- 10フレームの投球数がスペアで4投
- 10フレームの投球数がストライクで2投
ホタ「こういうバリデーション系は境界値チェックを意識するといいよ。あとはされたら困ること
を考える。その値持って来られたらエラーになって処理止まっちゃうんだよな〜とかあるじゃん。そういう要素を着実に潰すことを意識しよう」
私「なるほど……」
ホタ「あとRuboCop
そんな怒られんで」
私「うそ」
ホタ「ガチ」
ホタ「私ちゃんめっちゃraise
してたけどそもそもraise
、1箇所でいいよね」
私「はえ、」
ホタ「バリデーション全部まとめてやったらいいのでは?」
私「…………………………………………………………………………」
私「!!!!!!valid_score?
の中で全部判定して、最終的にboolean
が返れば、false
の時にエラーをraise
出来る……!」
ホタ「そういうこと。」
ホタ「最初に嫌なデータが入ったものは除外しておけば、そこ以降は嫌なデータについて考慮する必要がなくなる。」
ホタ「私ちゃんは都度弾いてたけど、都度弾くのって複雑だし、常に嫌なデータが入っているか考慮をしないとダメじゃん。それって面倒だし大変じゃない?」
私「大変でした…………………………。」
ホタ「でしょ?処理速度、パフォーマンスに問題がないとき
は最初に弾いてしまうといいよ。引数で渡されたものを2回見てるから冗長っちゃ冗長って意見もあるけど。」
私「なるほでょ」
ホタ「あとカスタム例外は最初に定義しよう。定数と同じ感覚ね。私ちゃんみたいに下に定義すると、コードが長い時に『どこにあんの?』ってなっちゃうから。」
私「わかりました!」
※ホタルイカ先輩のソースだとバリデーションのチェックは以下のようになります(計算ロジックは後ほど出るので一旦割愛してます)
class Bowling
class InvalidScore < StandardError; end
def initialize(score)
@score = score
end
def result
raise InvalidScore unless valid?
# @score.each_with_index.inject(0, &method(:frame_score))
end
private
def valid?
return false unless valid_type?
return false unless valid_pitching_count?
true
end
def valid_type?
return false unless @score.is_a? Array
return false unless @score.all? { |score| score.is_a? Array }
return false unless @score.all? { |score| score.all? { |s| (0..10).to_a.include?(s) } }
true
end
def valid_pitching_count?
return false if @score.size != 10
return false unless @score[0..8].all? { |score| score.size == (strike?(*score) ? 1 : 2) }
return false unless valid_last_pitching_count?
true
end
def valid_last_pitching_count?
return false if @score[9][0..1].sum >= 10 && @score[9].size != 3
return false if @score[9][0..1].sum < 10 && @score[9].size != 2
true
end
def strike?(*frame)
frame[0] == 10
end
def spare?(*frame)
return false if frame[2]
return false if frame[1].nil?
frame[0] + frame[1] == 10
end
end
ホタ「ちなみに私ちゃんの実装だと小数点弾けてなかったよ。」
私「ヴェ」
ホタ「0..10
って小数点も含まれるから。」
私「整数しか含まれないと思ってました……。」
ホタ「0〜10の整数ってやりたいなら.to_a
しなきゃだったね。」
私「あ……(頭をかかえる)」
・
・
・
ホタ「今回Ruby
だからinclude?
でいいと思うけど、Rails
ならin?
ってメソッドが使える。include?
とはレシーバと引数が入れ替わるから、レシーバがnil
じゃないってことを担保しないといけなくなるけど、短く書ける。覚えておくといいよ」
私「知りませんでした……in?
か、なるほど……。」
・
・
・
ホタ「eachの中で10フレーム目で計算結果returnしてるのも分かりにくい。イメージとしてはループの中で計算処理をやって、最後にループの外で答えが返る感じだよね。」
・
・
・
ホタ「計算メソッドにも関わらずnil
が返っちゃうメソッドもいくつかあったよね。計算メソッドは数字が返る
はず。そのメソッドがどんなメソッドで、返り値が何かを意識して実装しよう。」
ホタ「これはこういうメソッドです
って自分が書いたメソッドを一回説明してみるといいと思う。そうすると今言ったことが意識できると思う。」
・
・
・
ホタ「あと、純粋に合計を出す時はinject
使うかな。私ちゃんの実装だとtotal_score
って変数に合計を足してるよね。でもinject
ならその変数を使わない。何がいいかっていうと変数の名前を考える必要がなくなる
。」
ホタ「変数の名前を付けるのって大変だし、極力変数は増やさない方がいい。」
私「変数名考えるの苦手です……」
ホタ「でしょ?」
私「(inject
、謎の苦手意識があるから、これからはもっと意識して使おう……。)」
・
・
・
ホタ「あと計算結果を返す部分、俺のソースだと @score.each_with_index.inject(0, &method(:frame_score))
って書いてるでしょ?」
私「そのなんかやばいやつ何ですか」
ホタ「Enumerator
が返るメソッドはこういう形でチェーンできる。」
私「はええ」
⭐︎仕組みについての参考記事はこちら
(ちょっとまだ頭の中で整理できてないので後日まとめます)
※以下、ホタルイカ先輩の完全版ソースコード
class Bowling
class InvalidScore < StandardError; end
def initialize(score)
@score = score
end
def result
raise InvalidScore unless valid?
@score.each_with_index.inject(0, &method(:frame_score))
end
private
def frame_score(sum, (frame, i))
subtotal = frame.sum
subtotal += strike_calculation(i) if strike?(*frame)
subtotal += spare_calculation(i) if spare?(*frame)
sum + subtotal
end
def strike_calculation(index)
return 0 if index == 9
total = @score[index + 1][0, 2].sum
return total if index == 8 || !strike?(*@score[index + 1])
total + @score[index + 2][0]
end
def spare_calculation(index)
return 0 if index == 9
@score[index + 1][0]
end
def valid?
return false unless valid_type?
return false unless valid_pitching_count?
true
end
def valid_type?
return false unless @score.is_a? Array
return false unless @score.all? { |score| score.is_a? Array }
return false unless @score.all? { |score| score.all? { |s| (0..10).to_a.include?(s) } }
true
end
def valid_pitching_count?
return false if @score.size != 10
return false unless @score[0..8].all? { |score| score.size == (strike?(*score) ? 1 : 2) }
return false unless valid_last_pitching_count?
true
end
def valid_last_pitching_count?
return false if @score[9][0..1].sum >= 10 && @score[9].size != 3
return false if @score[9][0..1].sum < 10 && @score[9].size != 2
true
end
def strike?(*frame)
frame[0] == 10
end
def spare?(*frame)
return false if frame[2]
return false if frame[1].nil?
frame[0] + frame[1] == 10
end
end
※ホタルイカ先輩は配列の要素番号に合わせて0からカウントしてた
※配列の要素番号としてindex
を使う場合は0からカウントしよう!
フェーズ2まとめ
- 境界値チェックを意識する
- 自分の実装するものがどんな風に使われるのか?どんな値が与えられそうか?どんな値が来たらエラーになるか?を意識しながら実装する
- 異常なデータは処理を実行する前に全部弾く
- カスタム例外はクラスの頭で定義する
- 範囲オブジェクトを使って、整数のみ扱いたい時は
.to_a
する。(整数のみの配列にする) -
Ruby
=>include?
,Rails
=>in?
- 足し算するなら
inject
- メソッドは役割と返り値を意識する
- 配列の長さを見たいだけなら
size
を使う -
Enumerator
クラスを返すメソッドはチェーンできるしブロック渡せる(やばい)
最後に
今回は例外パターンの洗い出しが苦手だなということを実感できる課題でした。どんな値が来るのか?というイメージが不足していました。この手の洗い出しは普段から苦手とするところのため、レビューいただいた箇所を意識して改善します。
計算ロジック自体は問題なく組めていたので、あとはメソッドをどう書くか?という部分ですね。
inject
に対する謎の苦手意識を払拭して使いこなしたいと思います。