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

Caroline Nardi - JS Scrabble - Octos #34

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

Conversation

cmn53
Copy link

@cmn53 cmn53 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 approach used to determine things like highestScoreFrom, highestScoringWord, and highestWordScore was all directly translatable from the Ruby Scrabble project.
What was a challenge you faced in this assignment? I went back and forth with how to handle some of the potential invalid inputs and the level of detail that should be included in the error messages, ie. is it better to group them all into one line and throw a less specific exception or have 20 lines of code to cover every potential invalid input and give the user a more specific message. I was also looking for an opportunity to pass a function as an argument to another function, but none were readily apparent (and/or would have involved changing the provided tests).
Do you have any recommendations on how we could improve this project for the next cohort?
No, it seemed pretty straight-forward and I liked repeating a project we had done in another language, both because we could use it as a reference for the game logic, and to see how far we have come since then.

@droberts-sea
Copy link

JS Scrabble

What We're Looking For

Feature Feedback
Core Requirements
Git hygiene I would like to see more frequent commits.
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 Excellent work overall!


const Scrabble = {
score(word) {
if (!word || typeof word !== 'string' || word.length > 7) {
throw new Error('Invalid input');
}

Choose a reason for hiding this comment

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

I like that you're throwing full instances of Error here. Throwing raw strings is tempting, but you don't get a full stack trace.

tieBreaker(first, second) {
if (first.length === 7) {
return first
} else if (second.length === 7) {

Choose a reason for hiding this comment

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

Good use of a helper function to clarify this logic

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