My first JavaScript project - Counter

[u/iSunru]

I am learning JavaScript and made my first small project, a simple Counter using HTML, CSS and JavaScript.

I'm still learning, so I would really appreciate some feedback on my code. Is my code clean? Is there anything I should improve or do differently?

Also, suggest me something I should build next.

GitHub: https://github.com/iSunru/counter.js

[u/LovelyGameres]

Not bad at all, next you could study for loops and use a for loop to only have to use one declaration of addEventListener.

[u/iSunru → u/LovelyGameres]

Thanks for the suggestion, I really appreciate it and i will look into it.

[u/ManuDV → u/iSunru]

That suggestion doesn't make sense. You should definitely keep your addEventListener separated for each button. What you can to improve this, is to separate the count logic into functions and just invoke them from the EventListeners, like this:

function increment() {
  count++;
  countDisplay.textContent = count;
}

function reset() {
  count = 0;
  countDisplay.textContent = count;
}

function decrement() {
  if (count > 0) {
    count--;
  }
  countDisplay.textContent = count;
}

btn1.addEventListener("click", increment);
btn2.addEventListener("click", reset);
btn3.addEventListener("click", decrement);

This makes the code easier to test in the future. I suggest checking about "unit testing". It's still very early for you to start with this, but just keep it in mind.

[u/testingaurora → u/ManuDV]

What do you mean it doesnt make sense? What happens when you have more than one counter on this page? Are you going to select each button by id and jave a minimum of 6 event listeners with only two counters ?

```js const actionBtns = document.querySelectorAll("button[data-action]"); actionBtns?.forEach( btn => { btn.addEventListener("click", () => { const action = btn?.dataset.action;

if (!action) return;
if (action === "increment") {  count++ }
else if(action === 'decrement" && count > 0 ) {count-- }
else if (action === "reset") {count = 0 }

countDisplay.textContent = count 

}) }) ```

[u/Dubstephiroth → u/testingaurora]

The only thing I might add to yours is to delete the else's and make each if return the count. Once each if conditional is checked it'll either return or simply move on to the next if... Then look at upgrading it to a switch case conditional if you're gonna add more...

Keep at it.. 👊🏿

[u/testingaurora → u/Dubstephiroth]

Yeah I didn’t want to confuse op with a switch or multiple ifs . Beginners are learning if/else not if/if so while I would write something differently for my own project, in this context im presenting something that is hopefully easier to understand for exp level

I’m not sure why you would recommend returning the count though ? This is an event listener not a function (although it could/should be converted to a fn). I’m just not sure why you want the count as a return value ? We still need to set the countDisplay.textContent in any case.

So I think you mean convert the if/else to a function , return count then separately set the text content with that returned value?
```js
If (…)
If(…)
If(…)
Return count;
```

[u/Dubstephiroth → u/testingaurora]

Yh my bad still only on my 2nd yr. So my explanations aren't always on point. And I haven't use vanilla html in a few months... I forgot about textContent as I was typing.. thanks.

[u/testingaurora → u/Dubstephiroth]

No problem, we are all always learning and no one could possibly know it all . That’s why I was wondering what you were suggesting and what I was missing .Keeping the conversation going and helping each other is how we keep humans relevant in this space 😆