Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ampers - Sara S #31

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Ampers - Sara S #31

wants to merge 5 commits into from

Conversation

CheerOnMars
Copy link

@CheerOnMars CheerOnMars commented May 18, 2018

JS Scrabble

Congratulations! You're submitting your assignment!

Comprehension Questions

Question Answer
What patterns were you able to use from your Ruby knowledge to apply to JavaScript? The patterns I've noticed most were adding
What was a challenge you faced in this assignment? Besides keeping track of () and {}, it took some thinking through to get my head around 'this.'
Do you have any recommendations on how we could improve this project for the next cohort?

@CheezItMan
Copy link

JS Scrabble

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene You should have more commits. The commit messages are good.
Comprehension questions Check, it' normal to have issues with this.
General
score calculates score, has appropriate params and return value Check
highestScoreFrom calculates highest scoring word, has appropriate params and return value Check
Player object
Has name and plays properties Check
Has play, totalScore, hasWon functions Check
Has highestScoringWord and highestWordScore functions Check
Overall Well done you hit all the learning goals, well done!

let letters = word.toUpperCase().split(``);

if (word.match(/^[a-zA-Z]{1,7}$/) == null) {
throw `How'd you get a non-letter tile, even?`;

Choose a reason for hiding this comment

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

:)

}

hasWon() {
if (this.totalScore() >= 100) {

Choose a reason for hiding this comment

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

You could also simply

return this.totalScore() >= 100;



test('returns false when score < 100', () => {
const player = new Scrabble.Player('test player');

Choose a reason for hiding this comment

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

It would be good to loop through until you reach 99, the edge case, and verify that hasWon() is false.



test('returns true when score > 100', () => {
const player = new Scrabble.Player('test player');

Choose a reason for hiding this comment

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

It would also be good to verify that 101 makes hasWon() true

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