Simple Counter Javascript

September 28, 2022

Today I’m working on making a simple counter with HTML / CSS and JS.

My javascript isn’t working and I don’t know why.. I read a lot on the internet and tried to add "async" but it seems that my code needs a review

let counter = document.getElementById('counter');
let increaseButton = document.getElementById('increase');
let decreaseButton = document.getElementById("decrease");
let resetButton = document.getElementById("reset");
let count = 0;

function functionIncrease() {
    count ++;
    counter.innerHTML(count);
    color();
}
function functionDecrease() {
    count--;
    counter.innerHTML(count);
    color();
}
function functionReset() {
    count = 0;
    counter.innerHTML(count);
    color();
}
function color() {
    if (count > 0) {
        counter.style.color = "green";
    } else if (count > 0) {
        counter.style.color = "red";
    }
}

increaseButton.addEventListener("click", functionIncrease());
decreaseButton.addEventListener("click", functionDecrease());
resetButton.addEventListener("click", functionReset())

<main>
    <h1>My Simple Counter</h1>
    <p id="counter">0</p>
    <div class="button">
        <button id="decrease">Decrease</button>
        <button id="reset">Reset</button>
        <button id="increase">Increase</button>
    </div>
</main>

I think it’s just a stupid error, but I really need your help !

Thank you!

>Solution :

There are many improvements to make. Clean and efficient coding will also resolve the issue at hand by itself.

Don’t add a single eventListener to every button itself that will call an independent function. Use querySelectorAll to address all buttons in one call. You get a Node-List which you can use the forEach iteration on.
Use an event delegation to check what element has been clicked. Then you can return the id of the clicked button with event.target.id.
Use a switch statement to either decrease, increase, or reset the counter instead of using independent functions with literally 2/3 of the same content.
Use textContent which is faster then innerHTMLas the DOm doesn’t need to be re-parsed. Also, it poses no security issue of a potential XSS Injection.
Instead of calling a new function to color the counter, simply use a ternary conditional operator.

let counter = document.querySelector('#counter'),
    button = document.querySelectorAll('.button button'),
    count = 0;

button.forEach(el =>
  el.addEventListener('click', function(event) {
    switch (event.target.id) {
      case 'decrease':
        count--;
        break;
      case 'reset':
        count = 0;
        break;
      case 'increase':
        count++;
        break;
    }
    counter.textContent = count;
    counter.style.color = (count > 0) ? 'green' : 'red';
  })
)

<main>
  <h1>My Simple Counter</h1>
  <p id="counter">0</p>
  <div class="button">
    <button id="decrease">Decrease</button>
    <button id="reset">Reset</button>
    <button id="increase">Increase</button>
  </div>
</main>