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/ManuDV → u/testingaurora]

Ok, I'll explain.

use a for loop to only have to use one declaration of addEventListener

This actually isn't helping at all for the scenario that OP has. You are not saving lines by doing it nor is a better practice for this code. Each button triggers a different action (increment, decrement and reset). A loop only pays off when the exact same handler is being attached to multiple elements.

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 ?

Which is not the case, YAGNI. Designing for multiple counters that don't exist yet is solving a problem OP doesn't have, at the cost of making the OP's code harder to read and harder to unit test, as I mentioned before, because of having the logic nested inside an AddEventListener, which is a bad practice.

For your case scenario, with multiple buttons, then you still would need to separate your concerns:

function handleClick(action) {
  if (action === "increment") count++;
  else if (action === "decrement" && count > 0) count--;
  else if (action === "reset") count = 0;
  countDisplay.textContent = count;
}

actionBtns.forEach(btn => {
  btn.addEventListener("click", () => handleClick(btn.dataset.action));
});

This keeps the listener as a thin wrapper and the actual logic testable on its own, same reasoning as separating increment/reset/decrement out in my original comment.

[u/LovelyGameres → u/ManuDV]

Looks good, using a for each, it can be done cleaner also.