OskarSollenberg / ITTT---Hangman-ver2

Hangman Game
https://hangmanversion2.netlify.app/
0 stars 0 forks source link

Review #1

Open eskilfogelstrom opened 1 year ago

eskilfogelstrom commented 1 year ago

Great job!

Some constructive feedback

I think this is an example of when not sticking to DRY might be better. I think these functions are so small and can just as easily written right in the code without a function that they end up making the code harder to read (since I have to check what this function does in order to understand it). https://github.com/OskarSollenberg/ITTT---Hangman-ver2/blob/de9f6406c41e8ffc53adc24cba22acb2f78562ea/script.js#L48

As a bonus they could also be written using the for ... of ... syntax to make it even shorter

for (let letter of allLetters) {
  letter.classList.add('visible');
}

Same with these. I think I'd prefer just having this written out directly as then it's obvious what happens instead of me having to check this function https://github.com/OskarSollenberg/ITTT---Hangman-ver2/blob/de9f6406c41e8ffc53adc24cba22acb2f78562ea/script.js#L69


This is a fairly advanced topic but in general I think it's nice to make something called side-effects more explicit. If you call this function it changes a global variable. However this is not clear just from calling the function. https://github.com/OskarSollenberg/ITTT---Hangman-ver2/blob/de9f6406c41e8ffc53adc24cba22acb2f78562ea/script.js#L87 Instead I'd do the assignment in here:

word = generateRandWord();

// ...

function generateRandWord() {
    return words[Math.floor(Math.random() * words.length)].toUpperCase();
}

https://medium.com/@akhilanand.ak01/the-power-of-pure-functions-in-javascript-writing-reliable-and-maintainable-code-fc8a376996f2

OskarSollenberg commented 1 year ago

Awesome!, Thank you so much for the feedback! :)