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

Leaves - Dominique #37

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

Leaves - Dominique #37

wants to merge 4 commits into from

Conversation

dtaylor73
Copy link

JS Adagrams

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? While loops
Did you need to use different strategies to find information online about JavaScript than you would for Ruby? Yes, I couldn't go to the ruby documentation
What was a challenge you were able to overcome on this assignment? The syntax
What has been interesting and positive about learning a new programming language? The syntax is different from Ruby, but it is interesting to learn about
What is something to focus on your learnings in JavaScript in the next week? Diving more into arrow functions

@beccaelenzil
Copy link

JS Adagrams

What We're Looking For

Feature Feedback
General
Answered comprehension questions tell me more -- what strategies did you use to find information online?
Small commits with meaningful commit messages yes
Code Requirements
drawLetters method yes
Uses appropriate data structure to store the letter distribution yes -- see comment about where to put this
All tests for drawLetters pass yes
usesAvailableLetters method
All tests for usesAvailableLetters pass yes
scoreWord method
Uses appropriate data structure to store the letter scores yes -- see comment about where to put this
All tests for scoreWord pass yes
Optional
highestScoreFrom method N/A
Appropriately handles edge cases for tie-breaking logic N/A
All tests for highestScoreFrom pass N/A
Overall Good work overall! In this project you’ve taken some interesting logic and worked it into JavaScript syntax. There's one small bug in the drawLetters() function that provides a nice opportunity to think critically about the algorithm. Let's talk about it when we meet! I’ve added a few comments for your review, but overall, you’ve done a great job of practicing syntax.

drawLetters() {
// Implement this method for wave 1
const rawTiles = {

Choose a reason for hiding this comment

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

Style wise - I recommend putting the object outside of the drawLetters() function (but inside the module).



scoreWord(word) {
const scoreChart = {

Choose a reason for hiding this comment

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

Style wise - I recommend putting the object scoreChart outside of the scoreWod() function (but inside the module).

Choose a reason for hiding this comment

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

Style note: You should create this object the same way you did for rawTiles:

const scoreChart = { A: 1, B: 3 ... }

}
}

input.split('').forEach(checkForLetter);

Choose a reason for hiding this comment

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

Consider whether there's a different type of for loop that you could use that would allow you to break out of the loop as soon as you encounter false.


const letterPool = [];

for(const key in rawTiles) {

Choose a reason for hiding this comment

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

I really like that you store the letter frequencies and then build the pool - this is much easier to read than a giant array full of 9 'A's, 2 'B's, etc.


let i = 0;

while ( i <= 9) {

Choose a reason for hiding this comment

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

It looks like you could get the same letter twice because there's nothing keeping the random number from being repeated. Consider how you could address this.

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