This is a multiplayer bowling simulator in Ruby. It allows for variable skill levels, and produces weighted random results for each player's rolls based on those skill settings.
This is a complete rewrite of code I first posted for review here. My first solution was entirely procedural (all methods, no classes). I got some great pointers on OOP basics in the response, plus a referral to Sandi Metz, and worked out a new solution on that basis.
Everything works correctly. I'm posting the core classes, but the complete code (with scoring procedures, user input/screen output, tests) is in this gist.
For review: I'm looking for critiques/advice regarding the PlayerGame class in particular (3rd block below), where I've located the primary game logic.
Two basic questions:
- Does this
PlayerGameclass qualify as 'single responsibility'? If not, what should go? - For a program this small, are the direct dependencies (see
.bowland.score_game- instance creation in each case) worth injecting or otherwise minimizing?
The game logic had to go somewhere, and PlayerGame seemed like the best place. The remaining classes are fairly well isolated/dumb. That means PlayerGame is directing instance creation; the dependencies are at least isolated in private methods, but it seems like I could/should do better.
I'm wondering about a) Moving instance creation outside the class entirely (I think this would involve learning more about factory patterns. In practice, though, would you typically bother with the extra layer in a program this small?); b) Removing the turn-by-turn score data from PlayerGame and encapsulating it in a different class (Or do frames-played and scores really belong in a common object?); and/or -- c) Rethinking the modeling entirely (Trying to get away from gameplay as instance creation?).
Design/modeling considerations:
- I'm defining the problem around a game that's scored in progress (as needed by the display at an alley, essentially), so this differs from the 'kata'/Robert Martin version of the problem.
- I'm modeling a
Gameas a collection ofPlayerGame(s), and modeling an individual player-game as a series of consecutiveFrameinstances with corresponding subtotals. Frameresults are generated byPlayerinstances (which store and apply skill levels). Scores are calculated within one-time-useScoringWindowinstances that are only aware of a given player's last three frames. KeepingFrameas a class means.strike?(etc.) is available both during gameplay and during scoring.- I'm modeling gameplay itself in terms of instance creation. Unlike card games (where you can generate a deck in advance, and gameplay is just re/distributing cards), it would seem odd to generate a bowler's frames in advance rather than on a turn-by-turn basis.
Player:
# This class creates arrays of weighted-random rolls (1 or 2, from 10 pins).
class Player
attr_reader :skill
def initialize(skill = 0)
raise RangeError unless skill >= 0
@skill = skill
end
def roll
reset_lane
weighted_roll(@pins_standing)
@results
end
private
def reset_lane
@roll_no = 1
@pins_standing = 10
@results = []
end
def update_lane(pins_hit)
@roll_no += 1
@pins_standing -= pins_hit
end
def weighted_roll(pins)
pins_hit = apply_skill(pins)
@results << pins_hit
update_lane(pins_hit)
weighted_roll(@pins_standing) unless (pins_hit == 10 || @roll_no > 2)
end
def apply_skill(pins)
picks = []
(@skill + 1).times { picks << rand(0..pins)
break if picks.max == pins }
picks.max
end
end
Frame:
# This class stores and evaluates a single array of rolls.
class Frame
attr_reader :results
def initialize(player_roll)
@results = player_roll
end
def first_roll
@results[0]
end
def second_roll
@results[1]
end
def total
@results.reduce(:+)
end
def strike?
@results[0] == 10
end
def spare?
strike? == false && total == 10
end
end
PlayerGame:
# This class generates and stores Frame objects, then sends for scoring.
class PlayerGame
attr_reader :player, :frames, :scores
def initialize(player)
@player = player
@frames = []
@scores = []
end
def take_turn
@frames.length == 9 ? bowl_tenth : bowl
score_turn
end
def frames_played
@frames.map { |fr| fr.results }
end
def scores_posted
@scores.flatten
end
private
def bowl
player_frame = Frame.new(@player.roll)
@frames << player_frame
end
def bowl_tenth
base_frame = Frame.new(@player.roll)
if base_frame.strike? || base_frame.spare?
tenth_frame = generate_bonus(base_frame)
@frames << tenth_frame
else
@frames << base_frame
end
end
# Covers all possible cases starting from strike or spare (including 10-10-10).
def generate_bonus(base_frame)
first_bonus = Frame.new(@player.roll)
second_bonus = Frame.new(@player.roll)
source_rolls = [base_frame.results, first_bonus.results, second_bonus.results]
three_rolls = source_rolls.flatten.shift(3)
Frame.new(three_rolls)
end
def current_frame
@frames.length
end
def last_known_score
@scores.compact.empty? ? 0 : @scores.compact[-1]
end
def score_turn
active_frames = (current_frame <= 3) ? @frames : @frames[-3..-1]
window = ScoringWindow.new(active_frames, last_known_score)
@scores << window.return_scores[-1]
@scores[-2] ||= window.return_scores[-2] if window.return_scores.length >= 2
@scores[-3] ||= window.return_scores[-3] if window.return_scores.length == 3
end
end
ScoringWindow:
Omitted here (see link above), but each instance generates/returns array of 1-3 elements, used by PlayerGame at end of preceding block.
Game:
# This class creates a single game and directs player(s) to bowl in sequence.
class Game
attr_reader :players, :player_games
def initialize(players, turn_recorder)
@players = players
@turn_recorder = turn_recorder
@player_games = []
@players.each { |player| @player_games << PlayerGame.new(player) }
play_game
end
private
def play_game
10.times { play_turn; record_turn }
end
def play_turn
@player_games.each { |curr_player| curr_player.take_turn }
end
def record_turn
@turn_recorder.record(@player_games)
end
end
