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

Brenda Rios - Scrabble - Octos #32

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

Conversation

brendarios
Copy link

@brendarios brendarios 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? I used similar conditional logic patterns.
What was a challenge you faced in this assignment? Learning syntax and dealing with the semi colon and curly braces
Do you have any recommendations on how we could improve this project for the next cohort? I liked the project, it was good to go back to my ruby code and then compare and see that my code in JS looks cleaner. I'm liking Javascript.

@droberts-sea
Copy link

JS Scrabble

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene yes
Comprehension questions yes
General
score calculates score, has appropriate params and return value yes
highestScoreFrom calculates highest scoring word, has appropriate params and return value yes
Player object
Has name and plays properties yes
Has play, totalScore, hasWon functions yes
Has highestScoringWord and highestWordScore functions yes
Overall Great work overall! I'm glad you're enjoying JavaScript. I've left a few nitpicks below, but in general this solution looks good.


} else {
throw "Word must only contain letters from A-Z and have less than or equal to 7 characters";
}

Choose a reason for hiding this comment

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

It might be a little cleaner to invert this conditional:

if (!(/^[a-zA-Z]+$/.test(word) && word.length <= 7)) {
  throw new Error("Word must only contain letters from A-Z and have less than or equal to 7 characters");
}
// ... lines 11 through 20 ...


} else {
throw "Word must only contain letters from A-Z and have less than or equal to 7 characters";
}

Choose a reason for hiding this comment

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

If you throw a regular string like you do on line 23, you don't get backtrace information! You should throw an instance of Error instead (i.e. throw new Error("Word must..."))

const winningWord = arrayOfWords.reduce((word1, word2) => {
const scoreWord1 = Scrabble.score(word1);
const scoreWord2 = Scrabble.score(word2);

Choose a reason for hiding this comment

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

Good use of reduce here!

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