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

Shari Scrabble #17

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

Shari Scrabble #17

wants to merge 12 commits into from

Conversation

SSBinks
Copy link

@SSBinks SSBinks commented Nov 18, 2016

I really enjoyed this exercise but was very stuck at the beginning. I think once I knew how to debug, re errors and somewhat figured out how variables and functions could interact, this made a bit more sense. I don't think my highest score logic in the scrabble object was the best (just like Scrabble OG) but it works. One thing I was happy about was being able to use an object for the score (scrabble tiles) and have the letters as arrays. In the original Scrabble, I was unable to figure the out and was happy with being able to figure out that logic this time around. If I had more time, I think I would have done TDD instead of just pushing through. I actually used my TDD examples to print out and test my scrabble methods.

I finished Wave II but was unable to move on to wave three.

Feedback on organization would be appreciated.

var num = Object.keys(Scrabble.scoreChart);
var letters = Object.values(Scrabble.scoreChart);
for (i = 0; i < upWord.length; i++) {
for (d = 0; d < num.length; d ++){

Choose a reason for hiding this comment

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

We generally use i, j, k for the iteration variable.

var upWord = word.toUpperCase();
var num = Object.keys(Scrabble.scoreChart);
var letters = Object.values(Scrabble.scoreChart);
for (i = 0; i < upWord.length; i++) {

Choose a reason for hiding this comment

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

Because you don't have the var in front of these iteration variables, each of these loops are creating global variables

}
}
}
if (word.length >= 7){

Choose a reason for hiding this comment

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

I might argue that greater than 7 would be an error not a bonus

});
for (var i = 0; i < scores.length; i++){
if (scores[i] === max){
if( (words[i].length === 7 && words[i] > maxWord) || ( words[i].length < maxWord.length && maxWord.length !== 7)){

Choose a reason for hiding this comment

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

This line would be a great place to add a comment

this.finalScore = 0;
};

function add(a, b) {

Choose a reason for hiding this comment

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

Is this left over as a test from something else?

Choose a reason for hiding this comment

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

Ohhh -- now I see how you're using this within the reduce logic

else {
this.scores.push(Scrabble.score(words));
this.totalScore();
return Scrabble.score(words);

Choose a reason for hiding this comment

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

By calling score in here twice, you are doing the scoring logic twice which is unnecessary

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants