Skip to content

Bowling challenge - #1604

Open
carolrs wants to merge 5 commits into
makersacademy:mainfrom
carolrs:main
Open

Bowling challenge#1604
carolrs wants to merge 5 commits into
makersacademy:mainfrom
carolrs:main

Conversation

@carolrs

@carolrs carolrs commented May 2, 2023

Copy link
Copy Markdown

No description provided.

@tomcarmichael tomcarmichael left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great - well done Ana! As we discussed could maybe have unit tests as the only improvement I can think of!

Comment thread bowling/frame.js
}

isStrike(){
return this.rolls[0] === 10

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice ! terse way to write

Comment thread bowling/frame.js
}

score(){
let sum = this.rolls.reduce((acc, number) => acc + number)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great use of reduce

Comment thread bowling/score.js

}

addFrame(...rolls){

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great

Comment thread bowling/score.js


} //spare
else if (currentFrame.isSpare() && this.hasNext(i)) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider breaking this out into a separate function as this function is a little long

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants