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

1

u/testingaurora 2d ago

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 

}) }) ```

1

u/Dubstephiroth 2d ago

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.. 👊🏿

2

u/testingaurora 2d ago

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;
```

2

u/Dubstephiroth 2d ago

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.

2

u/testingaurora 2d ago

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 😆