Skip to content

Comments

Thomas Jensen#218

Open
th0jensen wants to merge 3 commits intoboolean-uk:mainfrom
th0jensen:main
Open

Thomas Jensen#218
th0jensen wants to merge 3 commits intoboolean-uk:mainfrom
th0jensen:main

Conversation

@th0jensen
Copy link

No description provided.

Copy link
Contributor

@dogezen dogezen left a comment

Choose a reason for hiding this comment

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

all your code is missing comments - it makes it hard for me to review because I have to analyise each line of code and really think hard about what each line does

with comments you make other people's lives much easier in terms of understanding and reflecting on your code

src/scrabble.js Outdated
// write code here
const modifiers = ['{', '}', '[', ']']
let totalPoints
const points = {
Copy link
Contributor

Choose a reason for hiding this comment

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

when we have constants: variables storing contant data that is fixed:

const SCRABBLE_POINTS = { ... }

src/scrabble.js Outdated
z: 10
}

function scrabble(input) {
Copy link
Contributor

Choose a reason for hiding this comment

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

input doesn't tell me what the variable is: it would be better named as inputWord

}

function scrabble(input) {
totalPoints = 0
Copy link
Contributor

Choose a reason for hiding this comment

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

why is totalPoints defined outside the function and not created inside the function?

src/scrabble.js Outdated
@@ -1,5 +1,103 @@
function scrabble() {
// write code here
const modifiers = ['{', '}', '[', ']']
Copy link
Contributor

Choose a reason for hiding this comment

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

same as scrabble points: i'd name these as WORD_MODIFIERS

src/scrabble.js Outdated
}

if (modifiers.some((mod) => input.includes(mod))) {
square(input)
Copy link
Contributor

Choose a reason for hiding this comment

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

unsure what square() and bracket() do -> function names are ambiguous, they should be more descriptive about telling me what the function does

src/scrabble.js Outdated
z: 10
}

function scrabble(input) {
Copy link
Contributor

Choose a reason for hiding this comment

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

function name ambiguous, better: computeScrabbleScore

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