Yar

Posted on Oct 5, 2020

Feedback on my JavaScript project

#javascript #beginners #discuss

Hi there!

I'm making my first steps in learning javascript.

Would you like to take a look at my practice project and provide some feedback? 🙈

I wonder what do you think about the functions and variable names.
As well as the overall structure of the app.

It displays letters on the page and lets you practice typing.

Here is the link to the app https://type.ptifur.digital

And here is the source code
https://github.com/ptifur/javascript-practice

Top comments (6)

Alex Janke • Oct 5 '20

Congrats for getting the app out there!

I haven't checked the source code yet (will do that later) but a suggestion would be to automatically jump to the next "word" once you have finished the current one. I feel like the in-between-step of the site just saying "Well Done" and waiting for your input to start again is not needed.
Also, how about a mode/settings where you can choose to use real world words too? The source for the words could be a list of the 1000 most used words in the english language (I'm sure there are a ton of lists online available).

Alex Janke • Oct 5 '20 • Edited

just leaving some thoughts/opinions in an unordered list:

you use google analytics but don't have cookie banner telling users about this. read into GDPR compliance regarding collecting user data. Google: use google analytics gdpr compliant.
I'd move the <script> of your JS file to the end of <body>
I feel like you could reduce some of your .js files. For example the file setTheUI.js doesn't import anything or does anything fancy that it needs to be its own module. I'm sure you can find a different spot for it and get rid of the file entirely :).
Same goes for the function createBoxes in util.js. createBoxes is just a function in util but displayBoxes gets its own .js file? It's not all bad but just heavily inconsistent.
You use a lot of empty lines. If you use VsCode, look into the extension Prettier and set VsCode to format on save (can be found somewhere in the settings)

Example:

// pressletters.js

  // A lot of empty lines here

   // game over
   if (letters.length === 0) {

        // reset the state
        gameStarted = false

        button.disabled = false

        caption.innerHTML = 'Well done!'

        setTimeout(playSound, 500, audioGameOver)

    }    

    return gameStarted

}

// But here suddenly none
function playSound(sound) {
    sound.play()
}

This file could also be named more generic, something like userInput.js.

You're super optimistic that you always pass the correct variables to other functions for example:

import { newLetters, createBoxes } from './utils.js'

export function displayBoxes(letters, number) {

    // generate new letters
    letters = newLetters(number)

    // display boxes
    createBoxes(letters, game)

    return letters

}

At some point in here you should probably check if the input, letters and number are actually what they are supposed to be or if they even exist at all (aren't null/undefined). Something like

export function displayBoxes(letters, number) {
  if (!letters) return "oops...";
}

Some syntax "issues"

plus.addEventListener('click', () => {

    if (NEW_LETTERS < 6) { NEW_LETTERS++ }

    setnum.innerHTML = NEW_LETTERS

})

could be changed to

plus.addEventListener('click', () => {
    if (NEW_LETTERS < 6) NEW_LETTERS++;
    setnum.innerHTML = NEW_LETTERS;
})

You define NEW_LETTERS all in capslock probably because it's a global variable but the other globals aren't capitalized. Looking at script.js line 20 and 22.

Since the page is doing what you want it to do it's definitely not bad :) Especially not when just starting out, you did good. From here it's just a constant process of iterating through optimisations and cleanup. I'd start by installing some formatter like Prettier to get the overall format in every file the same. Then go over and think about how you can A: rename files to make them more generic and B: merge some files because you really don't need to create a new file for just a singular function if this function isn't accessed by several other modules.

Yar • Oct 5 '20

Hi! If there is room for one more question, here is where I'm stuck with this app 🙊

I started working in a single file. Then made attempt to break it into modules. And when I move those plus and minus event listeners to a separate file, I can't properly pass the NEW_LETTERS value back to the main app.

Created new branch to illustrate it github.com/ptifur/javascript-pract...

Perhaps you could guide me in the right direction 🤓

Alex Janke • Oct 5 '20

Easiest would be to write a function setNewLetters (choose whatever name) in your script.js that sets the value for you.

// in script.js
export function setNewLetters(newValue) {
  if (!newValue) return;
  newLetters = newValue;
}

So you can then call this function in your event listeners and update your variable this way. Skips passing values back and forth, back and forth.

Yar • Oct 5 '20

Thanks for the insight!

Didn't know you can call the functions both ways. Guess this will save me some time stumbling through tutorials 😜

Yar • Oct 5 '20

Thanks for an extensive review and encouragement!

That's superhelpful, I appreciate it!

DEV Community

Feedback on my JavaScript project

Top comments (6)

Read next

What is the Difference Between Expressions and Statements in JavaScript?

A Simple, mostly CSS code editor

Fix Bug Search and Pagination in Vue.js: Dev Wants to Chill But Bug Doesn't Let

GlobalErrorHandler: Catch the errors that falls through ErrorBoundary's fingers!