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

Suzannah #34

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

Suzannah #34

wants to merge 6 commits into from

Conversation

sckirk
Copy link

@sckirk sckirk commented Nov 18, 2016

By day two (Wednesday), I started enjoying this short project to get more familiar with JS syntax. Last night I was so grateful for console.log helping me "see" that I'd forgotten () and that's why my code wasn't executing how I expected it to.

This project is not DRY, in multiple places!

I've added in functionality to score any words containing one or more non Alpha characters as zero, though in an actual Scrabble game, only Alpha tiles are provided as options. Through adding this optional functionality, I learned that the forEach function behaves differently than a for loop in another way that I didn't expect. While I like being able to 'name' what I'm passing into the forEach function, I might try to use more for loops in the future to avoid confusion. :)

var Scrabble = function() {
// due to how the specs are written, this score table does not account for the 50 point bonus for 7-letter words...
this.scoreTable = [
[1, "A", "E", "I", "O", "U", "L", "N", "R", "S", "T"],

Choose a reason for hiding this comment

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

The way this is structured could use JS objects to increase clarity. Using different data at specific indexes can oftentimes be unreliable.

var scoreTable = this.scoreTable; //this local variable is necessary to bypass the error that otherwise would be thrown in line 18 because the scope of 'this' changes inside anonymous functions.

if (word.length > 7) {
return 0;

Choose a reason for hiding this comment

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

This might also be good to return 0 when the length is zero

var nonAlphaChar = 0;
var alphaChars = ["A", "E", "I", "O", "U", "L", "N", "R", "S", "T", "D", "G", "B", "C", "M", "P", "F", "H", "V", "W", "Y", "K", "J", "X", "Q", "Z"];

var scoreTable = this.scoreTable; //this local variable is necessary to bypass the error that otherwise would be thrown in line 18 because the scope of 'this' changes inside anonymous functions.

Choose a reason for hiding this comment

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

The most common way to get beyond this issue is to create a variable called self which is set equal to this. Then, later on when you expect to use this you can use self instead. Sometimes folks also use that or _this, but I personally find both of those sort of confusing.

// to handle "tie" situations, creating a new array with the words whose score is the greatestVal
var highestScoreWords = [];
for (var k = 0; k < arrayOfWords.length; k++) {
if (this.score(arrayOfWords[k]) === greatestVal) {

Choose a reason for hiding this comment

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

It would be more efficient to use the scores array here instead of re-calculating the word scores

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