Ruby ボウリング問題解いてみた。

はじめに

この記事は「ボウリングのスコアを計算する クラスを作る」という問題を先輩から出題していただき、それを自力で解き、先輩のレビューをいただき振り返る過程を纏めた記事です。
備忘録・振り返り的な要素が強い為、フランクな書き方をしておりますが、大目に見ていただければと思います。軽い読み物だと思って読んでください。
また解き方に稚拙な箇所もあるかと思いますがご容赦ください。指摘等は大歓迎です。励みになります。

登場人物

• 私

  • 社会人２年目PG。Rubyでコーディングし始めて１年とちょっと。うっかりポンコツ。最近Reactを書き始めた。前回の問題以来、each_with_objectとgroup_byと仲良くなった。

• コウイカちゃん

  • 私の心の中に住んでいるイカ。 腹黒い。イカだけに。助言をくれたりくれなかったりする。

• ホタルイカ先輩

  • コウイカちゃんの先輩。沖漬けにされるのが好き。コーディングが得意。

出題された問題

今回ホタルイカ先輩から出題された問題は以下です。
以下の要件に従って問題を解いていきます。

ボウリングスコア計算

問題

• ボウリングのスコアを計算
• 投球を完了した状態で最終的なスコアを算出する。

入力例

• 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投目
]

要求仕様

• 最終スコアを数値（整数）で返す
• 入力値が整数ではない、ボウリングのルール上計算出来ない値の場合、例外を返す（例外はカスタム例外を作成する）
• カレントフレームスコアシステムは適用しない（これまで通りの計算方法）

フェーズ１　自分で考える

イメージを膨らませる

私「コウイカちゃんきいて」
コウイカ「どうしたの私ちゃん」
私「すごい重要なことに気付いたの」

私「私ボウリングのスコアの計算方法知らない……（片手で収まる程度にしかボウリングをやったことがない）」

コウイカ「まじ？」

コウイカ「これだから隠キャは……。」

私「きっっっっっっっっっつ」

コウイカ「イカ界ではボウリングのスコア計算なんて呼吸の仕方と同じくらい当たり前のことだわ。」

私「そっかあ（「ボウリング　スコア　計算」　で検索）」

ボウリングのルール

• スペアを投げた場合
  • スペアの次の１投で倒した本数を加算する
• ストライクを投げた場合
  • ストライク後に投げた２投で倒した本数を加算する

私「な、なるほど？」
私「じゃあ ストライク・ストライク・ストライクって投げたとき、最初のストライクのスコアは30になるってことだね？」
コウイカ「そうよ。」
私「なるほど……じゃあストライクはストライクが連続した場合と連続しない場合を考慮しなくちゃいけないんだね。」

私「あ。最終フレームで三回投げられる時ってあるよね。あれってどんなときなの？」
コウイカ「最初の１投がストライク、もしくは１投２投がスペアだった場合、３投投げることができるわよ。」

私「なるほど〜！スコア計算はそんな感じね。一旦例外を起こさなきゃいけない条件をまとめてみようかな。]

• 引数に渡された配列に文字列が含まれている場合
• フレーム数が10を超えている
• １フレームの配列の合計が10にならない場合
• １〜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じゃない場合
• 配列に１〜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

# 略

私「う〜〜ん。あと１フレームの配列の合計が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フレーム以外で２回投げてないのはおかしいからね。」
コウイカ「なるほどね。ってことは１０フレーム目の時の配列の長さのことも考慮しないといけないわね？」
私「そだねい〜！じゃあ次計算ロジックだ！とりあえず、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

私「最初にも言ったけど、今回は計算が４パターンある。」

• ストライクが連続する場合（例：３投連続でストライク）
• ストライクの場合
• スペアの場合
• ストライクでもスペアでもない場合

私「本当は各calculateメソッドを呼ぶところで後置ifしたいんだけど（イメージは以下）、RuboCopに怒られるからメソッドの中で上記４パターンを判定する。条件に一致しない場合はnilが返っちゃうからnilが返る場合は０を足すようにするよ。」

  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

私「よしよし！これでいいんじゃないかな！！おやつ食べよ！」

・
・
・
・
・

コウイカ「何か忘れてるんじゃない？」
私「ほ？」
コウイカ「自分で言ってたわよ」

私「？？？？」

私「アッ！？！？！？！３投目のバリデーション！！！」
コウイカ「（せいかいのおと）」

私「ええっと……３投目でありえそうなことは……」

• 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メソッドの中に３つ書くとそれだけで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

フェーズ１まとめ

• 場合分けは一通り最初に考えておく（今回はテストファーストでやったので、いつもに比べてコーディングの手が止まることがなかった）
• 大枠から作っていくとイメージをつけやすい（今回は最低限の骨組みだけ作って、ちょっとずつ肉付けしていった）
• RuboCopのABCSizeを回避するためにも、同じようなジャンルの処理はメソッド単位でパーツ化する（今回は場合分けした単位でメソッド化していった）

フェーズ２　レビューをいただく

私「ホタルイカ先輩、レビューお願いします！！！」
ホタ「よし。」

ホタ「まず最初に、俺が用意したテスト６つ落ちました」
私「えっっっっっっっっっっ」
ホタ「計算ロジックの考え方は良かったけど、バリデーション系で落ちてた。考慮漏れがあったってことだな」
私「（絶望）」

※漏れていた考慮は以下

• 引数の型が不正
• 投球に小数が含まれている
• フレーム毎の投球数が不足している
• 10フレームの投球数がミスで３投
• 10フレームの投球数がスペアで４投
• 10フレームの投球数がストライクで2投

ホタ「こういうバリデーション系は境界値チェックを意識するといいよ。あとはされたら困ることを考える。その値持って来られたらエラーになって処理止まっちゃうんだよな〜とかあるじゃん。そういう要素を着実に潰すことを意識しよう」
私「なるほど……」

ホタ「あとRuboCopそんな怒られんで」
私「うそ」
ホタ「ガチ」
ホタ「私ちゃんめっちゃraiseしてたけどそもそもraise、1箇所でいいよね」
私「はえ、」
ホタ「バリデーション全部まとめてやったらいいのでは？」

私「…………………………………………………………………………」

私「！！！！！！valid_score?の中で全部判定して、最終的にbooleanが返れば、falseの時にエラーをraise出来る……！」
ホタ「そういうこと。」

ホタ「最初に嫌なデータが入ったものは除外しておけば、そこ以降は嫌なデータについて考慮する必要がなくなる。」

ホタ「私ちゃんは都度弾いてたけど、都度弾くのって複雑だし、常に嫌なデータが入っているか考慮をしないとダメじゃん。それって面倒だし大変じゃない？」
私「大変でした…………………………。」
ホタ「でしょ？処理速度、パフォーマンスに問題がないときは最初に弾いてしまうといいよ。引数で渡されたものを２回見てるから冗長っちゃ冗長って意見もあるけど。」
私「なるほでょ」
ホタ「あとカスタム例外は最初に定義しよう。定数と同じ感覚ね。私ちゃんみたいに下に定義すると、コードが長い時に『どこにあんの？』ってなっちゃうから。」
私「わかりました！」

※ホタルイカ先輩のソースだとバリデーションのチェックは以下のようになります（計算ロジックは後ほど出るので一旦割愛してます）

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って小数点も含まれるから。」
私「整数しか含まれないと思ってました……。」
ホタ「０〜１０の整数ってやりたいなら.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からカウントしよう！

フェーズ２まとめ

• 境界値チェックを意識する
• 自分の実装するものがどんな風に使われるのか？どんな値が与えられそうか？どんな値が来たらエラーになるか？を意識しながら実装する
• 異常なデータは処理を実行する前に全部弾く
• カスタム例外はクラスの頭で定義する
• 範囲オブジェクトを使って、整数のみ扱いたい時は.to_aする。（整数のみの配列にする）
• Ruby => include?, Rails => in?
• 足し算するならinject
• メソッドは役割と返り値を意識する
• 配列の長さを見たいだけならsizeを使う
• Enumeratorクラスを返すメソッドはチェーンできるしブロック渡せる（やばい）

最後に

今回は例外パターンの洗い出しが苦手だなということを実感できる課題でした。どんな値が来るのか？というイメージが不足していました。この手の洗い出しは普段から苦手とするところのため、レビューいただいた箇所を意識して改善します。
計算ロジック自体は問題なく組めていたので、あとはメソッドをどう書くか？という部分ですね。
injectに対する謎の苦手意識を払拭して使いこなしたいと思います。