Worked on bowling TDD#1
Conversation
snozza
left a comment
snozza
left a comment
There was a problem hiding this comment.
Very good work - a few comments :)
| BowlingGame.prototype.calculateSpareBonus = function(frameIndex){ | ||
| return this.rolls[frameIndex + 2]; | ||
| } | ||
| BowlingGame.prototype.calculateStrikeBonus = function(frameIndex){ |
There was a problem hiding this comment.
Functions/Methods should do one thing - This should only calculate the Strike bonus.
However, it is also checking what roll the person is at - refactor this out of the function.
Tip: A strike bonus calculation will always be the same, regardless of what roll it is.
| } | ||
| } | ||
| } | ||
| BowlingGame.prototype.roll = function(pins) { |
There was a problem hiding this comment.
the Roll method is doing too much - it should just roll, it should not be checking the number of pins.
Tip: This can be a single line function
| game.roll(9); | ||
| game.roll(1); | ||
| game.roll(10); | ||
| let score = game.score(); |
There was a problem hiding this comment.
This should be const - be consistent with use of consts/lets
| test('handle a bonus for spare', () => { | ||
| rollMany(2, 5); | ||
| game.roll(4); | ||
| let bonus = game.calculateSpareBonus(0); |
There was a problem hiding this comment.
This should be const - be consistent with use of consts/lets
| rollMany(1, 10); | ||
| game.roll(8); | ||
| game.roll(6); | ||
| let bonus = game.calculateStrikeBonus(0); |
There was a problem hiding this comment.
This should be const - be consistent with use of consts/lets
| this.rolls.push(pins); | ||
| } | ||
| } | ||
| BowlingGame.prototype.score = function() { |
There was a problem hiding this comment.
You should be able to use this method to calculate the score without checking what index the roll is -
Think about iterating through frames instead of rolls.
| index++; // move to the next frame | ||
| } else if (this.isSpare(this.rolls[index], this.rolls[index + 1])) { | ||
| //add the two rolls in the current frame and the bonus for spare | ||
| sum += this.rolls[index] + this.rolls[index + 1] + this.calculateSpareBonus(index); |
There was a problem hiding this comment.
Repetition - anytime you see the exact same thing 2 or more times, you should think about whether or not it should be moved into a separate function (or variable if it is a variable)
| index += 2; // move to the next frame | ||
| } else { | ||
| //add the two rolls in the current frame | ||
| sum += this.rolls[index] + this.rolls[index + 1]; |
There was a problem hiding this comment.
Repetition - anytime you see the exact same thing 2 or more times, you should think about whether or not it should be moved into a separate function (or variable if it is a variable)
2 participants
I have done the bowling TDD assignment