Partially completed bowling challenge #394
Conversation
…sses created to allow this
…sses created to allow this, starting on roll method
…esults and started README
There was a problem hiding this comment.
I think a good start!
A few general comments:
Moving forward, I think there needs to be a defined line between game and scorecard class, as in which one is adding the rolls into the frames and which one is interpreting those scores and including spares / strikes where relevant. Currently as the player takes shots, the game copies that information between multiple classes. Instead there could be a singular class / source which holds all the records of the player and each different class could apply their methods using the same source. This would help reduce the complexity when introducing methods to calculate strike / spare scores.
Current tests seem to be covering the basics and need to be expanded to cover edge cases and actual game simulations, but the project is still in development so I am sure you will add more as you go.
If you are incorporating UI, I would also suggest adding a few validators to check if the user enters invalid input (string inputs, negative inputs etc.).
|
|
||
| Count and sum the scores of a bowling game for one player. For this challenge, you do _not_ need to build a web app with a UI, instead, just focus on the logic for bowling (you also don't need a database). Next end-of-unit challenge, you will have the chance to translate the logic to Javascript and build a user interface. | ||
| To run the tests, run rspec from the command line. |
There was a problem hiding this comment.
Currently the test on scorecard_spec.rb line 38 fails on rspec, but I am assuming that is because it is still in WIP
|
|
||
| ### Strikes | ||
| The main class that controls the flow of the game. It has an `initialize` method that sets up the game with a new Frame and a new Scorecard. It also has a `roll` method which calls the roll method on the current frame and updates the Scorecard. The game continues until all frames are played. |
There was a problem hiding this comment.
I get the logic, but do we really need a Roll class? It seems like all it is doing right now is keeping an integer for a score, could we have not done that by just using actual integers in an array?
| def update_frame | ||
| @roll_count += 1 |
There was a problem hiding this comment.
does this need to be a method seeing as it is a one-line code which is only used once?
| class Roll | ||
| attr_reader :score | ||
|
|
||
| def initialize(score) | ||
| @score = score | ||
| end | ||
| end |
There was a problem hiding this comment.
see the comment on readme, I think splitting the rolls into a different class over-complicates it
There was a problem hiding this comment.
It came about only from trying to make the program modular. Agreed it doesn't really suit this program though.
| @scorecard = Scorecard.new | ||
| @current_frame = Frame.new | ||
| @frames = [@current_frame] | ||
| @roll_count = 0 |
There was a problem hiding this comment.
I like the logic of splitting the scorecard in a different class, but it seems like you need to update the frames in scorecard class as well as your current game class, effectively duplicating the information. Can you not have a single array of frames record what the player has scored and all the classes could apply their logic on that one?
| def update(frame, frames) | ||
| frame.total = frame.rolls.sum(&:score) | ||
| if frame.rolls.size == 2 || frame.rolls.map(&:score).sum >= 10 | ||
| @frames << frame | ||
| end | ||
| end |
There was a problem hiding this comment.
see line 25 on game.rb, which is very similar. I think scorecard and game classes are slightly creeping into each other's territory, resulting in duplicating information / code
There was a problem hiding this comment.
Agreed. I will take the time to refactor the classes and ensure the functions are seperate.
| def to_s | ||
| scorecard_str = "" | ||
| score = 0 | ||
| @frames.each_with_index do |frame, index| | ||
| score += frame.total | ||
| scorecard_str += format_frame(index, frame, score) | ||
| end | ||
| scorecard_str | ||
| end | ||
|
|
||
| private | ||
|
|
||
| def format_frame(index, frame, score) | ||
| frame_info = "FRAME #{index + 1}:" | ||
| rolls_info = frame.rolls.each_with_index.map { |roll, r_index| "ROLL #{r_index + 1}: #{roll.score}" }.join(' ') | ||
| "#{frame_info} #{rolls_info} | TOTAL: #{frame.total} | SCORE: #{score}\n" | ||
| end | ||
| end |
There was a problem hiding this comment.
I like this portion of this class. It is good practice to keep this strictly to printing / displaying scores of the user
Need to work on bonus point implementation.