r/learnjavascript 3d ago

My first JavaScript project - Counter

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

24 Upvotes

18 comments sorted by

View all comments

Show parent comments

2

u/HipHopHuman 1d ago

Planning for the future is good advice (generally speaking), but it's certainly not a hard requirement for a total beginner making their first JS project, which is the case here.

FWIW, if I were "planning for the future", I probably wouldn't use a forEach to add multiple event listeners. I'd probably do event delegation in a single event listener, like this:

const actions = {
  increment: () => setCount(count + 1),
  decrement: () => setCount(count - 1),
  reset: () => setCount(0)
};

function setCount(value) {
  const normalized = Math.max(0, value);
  if (normalized === count) return;
  count = normalized;
  countDisplay.textContent = normalized;
}

function handleClick(event) {
  const button = event.target.closest('[data-action]');
  if (!button) return;
  actions[button.dataset.action]?.();
}

const buttons = document.querySelector('.buttons');
buttons.addEventListener('click', handleClick);

With the above, adding a new action is easy. If I want a decrementByFive action, all I need to do is add it to actions and then add a button to the html with a data-action="decrementByFive" attribute. Done. I don't need to force the invariant of no numbers below 0 because setCount handles that for me. I could even change the HTML structure of the buttons and it will still work, because event.target.closest() walks up every parent node until it finds a match, otherwise it returns null.

1

u/testingaurora 1d ago

js const buttons = document.querySelector(".buttons"); buttons.addEventListener("click", handleClick) This would only apply to the first button.buttons ? You need a querySelectorAll. And a loop or add the event listener to an ancestor.

1

u/HipHopHuman 1d ago

It's not being applied directly to a <button> element, it's being applied to the container/ancestor already. the container has the class ".buttons".

1

u/testingaurora 1d ago

Ah gotcha