Skip to content

Comments

core of challenge#222

Open
NadProgress wants to merge 1 commit intoboolean-uk:mainfrom
NadProgress:main
Open

core of challenge#222
NadProgress wants to merge 1 commit intoboolean-uk:mainfrom
NadProgress:main

Conversation

@NadProgress
Copy link

No description provided.

@@ -1,5 +1,55 @@
function scrabble() {
function scrabble(words) {
Copy link
Contributor

Choose a reason for hiding this comment

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

words is not correct, the variable represents 1 scrabble word so the name for the variable is better as word

function scrabble(words) {
// write code here
}
const dictionary = {
Copy link
Contributor

Choose a reason for hiding this comment

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

the name of this variable doesnt tell me what it represents / stores, a better name would be SCRABBLE_LETTER_SCORES since it is representing a fixed-value piece of data that you only read from. You can move it outside the function since it is constant and read-only.

Y: 4,
Z: 10
}
let fullTotal = 0
Copy link
Contributor

Choose a reason for hiding this comment

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

fullTotal variable name could improve because it doesnt tell me what it is, i don't know what full total means... perhaps wordTotalScore would be better or totalScore

return 0
}

if (words === null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

this if statement should be the first of the 3 that check whether the word is valid

}

for (let index = 0; index < words.length; index++) {
let wordsCapit = words[index].toUpperCase()
Copy link
Contributor

Choose a reason for hiding this comment

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

this variable is not correctly named: it isn't a word capitalised, it's the character capitalised...

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