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

js-scrabble #7

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

js-scrabble #7

wants to merge 24 commits into from

Conversation

Lisa-Sano
Copy link

Lisa Rolczynski

Lisa-Sano added 24 commits June 10, 2016 14:32
…rom a list. if tied, returns the one with 7 letters, or the shortest, or the one earlier in the list (in that order)
};

Scrabble.prototype = {
helloWorld: function() {return 'hello world!';},

Choose a reason for hiding this comment

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

👋🏻

@loganmeetsworld
Copy link

loganmeetsworld commented Jun 15, 2016

Lisa, fantastic first fore into Javascript OOP. Your objects are clean, have single responsibilities, and the code is clear. There were a couple conditionals that may get kind of tricky but overall looked good. The only other thing I have is that I'd like to see more comments/documentation about what the purpose of different methods are. The methods here are pretty clear but it's custom to have docstrings at the start of methods in the real world so it may be good to start doing more of that. Other than that, beautiful tests and awesome code!

Alt Text

@Lisa-Sano
Copy link
Author

Thanks so much @loganmeetsworld!

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