はじめに
毎月先輩から出していただいた課題に取り組んでいます、 mi0です。
1月はRuby ボウリング問題解いてみた。の機能拡張を行いました。(詳しくは出題された内容の欄をご覧ください!)
この記事は実装〜レビューをいただくまでの過程を纏めた備忘録です。
こうやったらもっとよくなる、などのご指摘があればコメント頂けると嬉しいです!
過去の記事はこちら!↓
- 「Ruby初心者向けのプログラミング問題を集めてみた」の電話帳問題解いてみた。
- Ruby ボウリング問題解いてみた。
- Ruby 価値が大きくなる組み合わせ問題 解いてみた
- Ruby初心者向けのプログラミング問題のカラオケマシン問題にハモリ機能を追加してみた
登場人物
- 私
- いよいよ本格的に年次が上がってしまうため焦り始めている。
- タラバガニ先輩
- 名前に王者の風格がある。プログラミングが得意。
- こっぺがにちゃん
- 私の心の中に住んでいる妖精。名前はカタカナではなくひらがなできちんと記載しないと怒られる。
出題された内容
ボウリングスコア計算 v2
問題
- ボウリングスコア計算機能に、フレーム時点でのスコアを計算する処理を追加する
- 10 フレーム投球を完了していない状態でも出力する
出力例
10 フレーム投球している場合
[
20, # 1フレーム目
39, # 2フレーム目
59, # 3フレーム目
79, # 4フレーム目
98, # 5フレーム目
118, # 6フレーム目
138, # 7フレーム目
157, # 8フレーム目
174, # 9フレーム目
194 # 10フレーム目
]
10 フレーム投球していない場合
[
19, # 1フレーム目
25, # 2フレーム目
55, # 3フレーム目
83, # 4フレーム目
102, # 5フレーム目
111, # 6フレーム目
121, # 7フレーム目
129, # 8フレーム目
nil, # 9フレーム目
nil # 10フレーム目
]
#### 要求仕様
- 既存(変更無し)
- 最終スコアを数値(整数)で返す
- 入力値が整数ではない、ボウリングのルール上計算出来ない値の場合、例外を返す(例外はカスタム例外を作成する)
- カレントフレームスコアシステムは適用しない(これまで通りの計算方法)
- 追加
- フレーム時点での点数を出力する
- 投球が途中(10 フレーム投ていない、1 投目のみ投球状態)であっても点数が出力できる
- フレームのスコアはフレーム時点で確定している場合、出力する
- その他
- 入力値チェックは、既存通り`ボウリングのルール上、計算出来ない値、状況の場合、例外を返す`
ベースとなるコード(主にdetailメソッドの実装を行う)
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
def details
[nil] * 10
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
フェーズ1 自分で考える
イメージを膨らませる
処理の流れを考えてみる
私「ざっくりと今回やりたいことってこんな感じだよね」
- フレームごとの合計点をだす(配列が返る)
- 計算できない時はnilを返す(ストライクの時・スペアの時)
- ストライクだが、ストライク後に2投していない場合
- スペアだが、スペア後に1投していない場合
- ストライクでないが、2投していない場合
- 不正な投球の場合は例外
- 最終フレームの投球数が不正
- 各フレームの投球数が超過している
こっぺがに「うんうん、そうね。投球が不正なパターンはもう少し細かく考える必要がありそうだけど、一旦は良さそう。」
こっぺがに「次に進みましょう!」
元々の実装を振り返る
私「前回の実装を踏まえて、現時点でどんなメソッドがあるか
・どんな処理になっているのか
を把握したいと思います!」
私「あるものを把握していれば、完全新規でメソッドを作らず、あるものを生かして実装できるので……前回の二の舞にはならないは!ず!!」
〜ここで過去記事と与えられたソースを振り返る〜
私「ボーリングのルール思い出す時最初からこれみたらよかったわ(普通にググって調べ直したかお)」
私「バカすぎわろち」
私「でもこれで把握した。」
私「今あるメソッドは大きく分けてこんな感じ」
今できること
トータルスコアの計算ができる
今あるメソッド
-
valid?メソッド
- 入力値チェック
-
valid_pitching_count?
- 10フレーム投げているかの判定
- 各フレーム毎の投球数が合っているか判定
-
valid_last_pitching_count?
- 最終フレームの投げた数のチェック
-
strike?
- ストライク判定してくれるメソッド
-
spare?
- スペア判定してくれるメソッド
-
frame_score
- トータルスコアを計算する
-
strike_calclation
- ストライク時のスコアを計算する
-
spare_calculation
- スペア時のスコアを計算する
不足要素
- 投球が不完全な場合、例外になる(valid?メソッド)
- 今回は最終フレームまで投げ切っていなくても得点を計算するため、上記のままでは仕様を満たせない
- 各フレームでの投球が計算できるかどうかを判定するメソッド
私「ふんふん、今までのvalid?
メソッドは10フレーム投げ切っていない
場合true
が返っていたから、その辺の判定を緩めた別のメソッドを定義する必要があるね」
こっぺがに「そうね、それから各フレームの投球数を確認して、計算できるかどうかをチェックするメソッドが今はないから、新しく追加する必要がありそう。ストライクとスペアでそれぞれ必要になりそうね。」
私「よし、なんとなくイメージが湧いてきた!コード書いてみよう」
実際にコードを書いていく
私「とりあえずざっくりとイメージをまとめてみよう。」
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
+ def details
+ raise InvalidScore unless valid_type? # 引数が不正だったら例外
+ raise InvalidScore unless valid_ten_frame? # 10フレーム目の投球が不正だったら例外
+ (0..9).inject([]) do |array, index|
+ # フレーム毎に計算を行う
+ end
end
# 中略
end
私「valid?
メソッドで使っていたvalid_type?
メソッドで入力値チェックする。あと今回新しくvalid_ten_frame?
メソッドを定義して、10フレーム目の投球のチェックをするよ。」
私「で、チェックして問題なければフレーム毎の合計点を計算する処理に入る!」
私「最初にvalid_ten_frame
メソッドを作ろう。」
class Bowling
class InvalidScore < StandardError; end
def details
raise InvalidScore unless valid_type?
raise InvalidScore unless valid_ten_frame?
(0..9).inject([]) do |array, index|
# フレーム毎に計算を行う
end
end
private
+ def valid_ten_frame?
+ return true if @score[9].nil?
+ return false if @score[9].size > 3
+ return false if @score[9][0..1].sum < 10 && @score[9].size != 2
+
+ true
+ end
end
私「10フレーム目で不正な場合は以下の場合だから、該当するときはfalse
を返す。」
- 10フレーム目を投げていない場合
- 10フレーム目で4投以上投げている場合
- 10フレーム目で1投しかしていないのに、10本以上ピンを倒している場合
こっぺがに「それ以外は一旦true
を返しておくのね」
私「そうです!!」
私「次!計算処理の部分!!!」
class Bowling
class InvalidScore < StandardError; end
def details
(0..9).inject([]) do |array, index|
+ next array << frame_total_score(index) if frame_total_score(index).nil?
+
+ array << (array.last || 0) + frame_total_score(index)
+ end
end
private
+ def frame_total_score(index)
+ return if uncalclatable_score?(index) # 計算できないスコアの場合
+ raise InvalidScore if invalid_total_score?(index) # 1フレームでのスコアが不正な場合
+ sum_score(index)
+ end
end
私「計算できないスコアの場合はnil
を返す仕様だから、nil
が配列に格納される。」
私「1フレームの特典がおかしいときは例外を発生させる。」
私「それ以外の場合は計算しておっけーなので合計を出す。」
私「まずは計算できないかどうか判定するuncalclatable_score?
メソッドを定義するよ!」
private
+ def uncalclatable_score?(index)
+ return true if @score[index].nil?
+ return true if !strike?(*@score[index]) && @score[index].size == 1
+ return true if uncalclatable_as_strike?(index) || uncalclatable_as_spare?(index)
+ false
+ end
+ def uncalclatable_as_strike?(index)
+ return false unless strike?(*@score[index]) || (index + 1) == 9
+ return false if !@score[index + 1].nil? && @score[index + 1].size == 2
+
+ throw_once_after_strike?(index) || no_throw_after_two_strikes?(index)
+ end
+ def uncalclatable_as_spare?(index)
+ spare?(*@score[index]) && @score[index + 1].nil?
+ end
+ def throw_once_after_strike?(index)
+ !@score[index + 1].nil? && !strike?(*@score[index + 1]) && @score[index + 1].size == 1
+ end
+ def no_throw_after_two_strikes?(index)
+ (index + 2) < 10 && @score[index + 2].nil?
+ end
私「計算できない場合を最初に挙げていたものよりも更に掘り下げたよ」
私「具体的には以下の場合!それらを網羅した実装をしたのが上のメソッド達だよ!」
- フレームで投球を行っていない
- ストライクではないが、1投しかしていない場合
- ストライクとして計算できない場合
- ストライクではない、あるいは10フレーム目の場合
- 投球は行っているが、2投している場合
- ストライクの後1投しかしていない場合
- 2連続ストライクの後1投していない場合
- スペアとして計算できない場合
- スペアの後に1投していない場合
私「次!!!invalid_total_score?
作ります!」
private
+ def invalid_total_score?(index)
+ @score[index].sum > 10 && index != 9
+ end
こっぺがに「これはこのまんまね」
私「そうね……10フレーム以外で合計点が10以上だったらダメですって判定をしているだけだからね……。」
こっぺがに「終わっちゃったわね……」
私「や、やったあ!!」
※実際はストライクの条件でもっと苦しみました
※コードの全貌は以下!
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
def details
raise InvalidScore unless valid_type?
raise InvalidScore unless valid_ten_frame?
(0..9).inject([]) do |array, index|
next array << frame_total_score(index) if frame_total_score(index).nil?
array << (array.last || 0) + frame_total_score(index)
end
end
private
def frame_total_score(index)
return nil if uncalclatable_score?(index)
raise InvalidScore if invalid_total_score?(index)
sum_score(index)
end
def valid_ten_frame?
return true if @score[9].nil?
return false if @score[9].size.to_i > 3
return false if @score[9][0..1].sum < 10 && @score[9].size != 2
true
end
def invalid_total_score?(index)
@score[index].sum > 10 && index != 9
end
def uncalclatable_score?(index)
return true if @score[index].nil?
return true if !strike?(*@score[index]) && @score[index].size == 1
return true if uncalclatable_as_strike?(index) || uncalclatable_as_spare?(index)
false
end
def uncalclatable_as_strike?(index)
return false unless strike?(*@score[index]) || (index + 1) == 9
return false if !@score[index + 1].nil? && @score[index + 1].size == 2
throw_once_after_strike?(index) || no_throw_after_two_strikes?(index)
end
def throw_once_after_strike?(index)
!@score[index + 1].nil? && !strike?(*@score[index + 1]) && @score[index + 1].size == 1
end
def no_throw_after_two_strikes?(index)
(index + 2) < 10 && @score[index + 2].nil?
end
def uncalclatable_as_spare?(index)
spare?(*@score[index]) && @score[index + 1].nil?
end
def frame_score(sum, (_frame, i))
sum + sum_score(i)
end
def sum_score(index)
subtotal = @score[index].sum
subtotal += strike_calculation(index) if strike?(*@score[index])
subtotal += spare_calculation(index) if spare?(*@score[index])
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
フェーズ2 レビューをいただく
タラバ「じゃあレビューしていくよ」
私「お願いします!(タラバガニ先輩、タラバガニだけあってオーラが違うな)」
タラバ「まずね〜」
私「はい(さながらローランドのような?風格が……)」
タラバ「俺が用意したテストが落ちたんだよね」
私「えっ!?!?!?!?!?!?」
タラバ「声でか」
私「嘘ですよね?????こ、心当たりが、」
タラバ「事実です。はい、これが落ちたテストの条件の一覧ね。」
- 投球フレーム数が超過している場合
- 11フレーム分投げている、など
- フレ-ム毎の投球数が不足している場合
- 3フレーム投球完了時、投球数が超過している場合
- 10フレームの投球数がストライクで2投の場合
- 10フレーム目の1投目がストライクの場合、10フレーム目は3投したタイミングでスコアが計算できる
私「言われてみたら10フレーム目の投球数の判定とかしてなかったかも……。」
タラバ「でしょ?」
タラバ「計算出来ない値
、状況の場合やフレームのスコアはフレーム時点で確定している場合
のチェックが甘かったように感じたから、今回みたいな場合もテストファーストで有り得る条件を全て網羅した上で、それをクリアしていくように実装するとよかったかな」
私「」
タラバ「次、ソースコードについて!」
タラバ「私ちゃんが書いたこの条件をみて欲しい」
@score[index].sum > 10 && index != 9
タラバ「別に条件として問題があるわけじゃないんだけど、より可読性をよくするためのアドバイスを一つ」
タラバ「条件式を書くときは日本語に変換した時に、日本語として綺麗にまとまっている
ような条件を意識するといいよ」
私「日本語として綺麗にまとまっている。」
タラバ「そう。日本語として綺麗にまとまっているってことは、コードを読む人が理解しやすい
ってこと。つまり可読性が高いコード
ってことだよね。」
私「確かに……!」
タラバ「今回の私ちゃんのコードを日本語にすると、合計点が10点以上の時、かつ10フレーム目の場合
だよね」
タラバ「でもボーリングってそれぞれフレームがあって、フレーム単位で投球を行う
でしょ?」
タラバ「だから 10フレーム目で、かつ合計点が10点以上の場合
の方が、日本語としてスッと入ってこない?」
私「入ってきますね……(RSpec書くときもそう書くだろうな……)」
タラバ「そういうところを意識するとこれからもっとコードがよくなると思うので、意識してやってみよう」
私「はーい!!」
タラバ「それじゃあ俺が書いたコードもお披露目しよっか」
私「!!!はい!!」
class Bowling
class InvalidScore < StandardError; end
include Validations
def initialize(score)
@score = score
end
def result
raise InvalidScore unless valid?
@score.each_with_index.inject(0, &method(:frame_score))
end
def details
raise InvalidScore unless valid_details?
provisional_score = 0
10.times.map do |i|
next if provisional_score.nil?
provisional_score = frame_score(provisional_score, [@score[i], i])
end
end
private
def frame_score(sum, (frame, i))
return if frame.nil?
return unless calculatetable?(i, frame)
subtotal = frame.sum
subtotal += strike_calculation(i) if strike?(*frame)
subtotal += spare_calculation(i) if spare?(*frame)
sum + subtotal
end
def calculatetable?(index, frame)
if strike?(*frame)
strike_calculatetable?(index)
elsif spare?(*frame)
spare_calculatetable?(index)
else
miss_frame_calculatetable?(index)
end
end
def strike_calculatetable?(index)
@score[(index + 1)..-1].flatten.size >= 2
end
def spare_calculatetable?(index)
!@score[index + 1].nil?
end
def miss_frame_calculatetable?(index)
@score[index].size == if index == 9 && (spare?(*@score[index][0..1]) || strike?(*@score[index][0..0]))
3
else
2
end
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 strike?(*frame)
return false if frame[1]
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
私「な、なるほど〜〜〜〜〜」
私「#calculatetable?
とかさっきのタラバガニ先輩の日本語に変換して綺麗にまとまっている
っていうのを体現してるかのようなコードですね……!そのままスッと日本語に変換して読めました。」
私「なるほどな〜……!」
学んだこと
- 条件が多く複雑な場合は、条件を網羅するために、テストファーストで実装を行う
- 日本語に変換した時に、日本語として綺麗にまとまっているコード=可読性が高いコードのため、日本語にした時に素直に読めるコードを書くことを意識することが大事
まとめ
今回の実装は割と手応えがあったのですが、条件の洗い出しが甘かったこともあり、先輩の用意したテストがいくつか落ちてしまいました……。悔しかったです。
可読性の高いコードを書く
というのが私のここ最近の私自身の課題だと思っているのですが、日本語にした時に分かりやすい
というのはなるほどなと思いました。
よく先輩が「こういうことをしたい
という理想形を一回日本語でメモ書き
してから実装するといいよ」と教えてくださるのですが、そのメモ書きの時点で日本語としてある程度綺麗にまとまっていれば、実際にコードを書く際、それに準じた形でコードを書けば可読性の高いコードが書けそうです。
意識してやってみたいと思います!