Simple Counter Javascript

Question

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!

counter.innerHTML is not a function, but a scalar property: counter.innerHTML = count. Though since count is not HTML, textContent would be better: counter.textContent = count. EDIT: Also what Luis said. — Amadan
– Amadan, Commented Sep 28, 2022 at 8:05
As the error says;innerHTML is not a function- its a property....so counter.innerHTML = "Foo"; — Jamiec
– Jamiec, Commented Sep 28, 2022 at 8:05

ATP · Accepted Answer · 2022-09-28 08:07:37Z

As your error says innerHTML is not a function, it's a variable.
change it to =

And remove the brackets of the listeners, you should pass the function reference not call it.

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>

tacoshy · Accepted Answer · 2022-09-28 08:23:01Z

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>

Nitheesh · Accepted Answer · 2022-09-28 08:07:27Z

Issues Found

Dont call the listener function while registering the event listner. Just mention the function reference. Just like increaseButton.addEventListener("click", functionIncrease);
Also the innerHTML should be set with counter.innerHTML = count

Fixed Code

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>

rrr63 · Accepted Answer · 2022-09-28 08:07:45Z

On your addEventListener, you have to change with functions names.

Also the .innerHTML was wrong : counter.innerHTML(count); to counter.innerHTML = count;

var counter = document.getElementById('counter');
var increaseButton = document.getElementById('increase');
var decreaseButton = document.getElementById("decrease");
var resetButton = document.getElementById("reset");
var 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)

<!DOCTYPE html>
<html>
    <head>
        <title>Simple Counter</title>
        <meta name="viewport" content="width=device-width, initial-scale=1" />
        <link href="styles.css" rel="stylesheet">
        
    </head>
    <body>

        <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>
        
        <script src="./script.js" async></script>
    </body>
</html>

Radheesh · Accepted Answer · 2022-09-28 08:32:03Z

-1

let counterDisplayElem = document.querySelector('.counter-display');
let counterMinusElem = document.querySelector('.counter-minus');
let counterPlusElem = document.querySelector('.counter-plus');
let reset=document.querySelector('.reset');
let count = 0;

updateDisplay();

counterPlusElem.addEventListener("click",()=>{
    count++;
    updateDisplay();
    color();
}) ;

counterMinusElem.addEventListener("click",()=>{
    count--;
    updateDisplay();
    color();
});

reset.addEventListener("click",()=>{
    count=0;
    updateDisplay();
});

function updateDisplay(){
    counterDisplayElem.innerHTML = count;
};

function color() {
    if (count > 0) {
        counterDisplayElem.style.color = "green";
    } else if (count < 0) {
        counterDisplayElem.style.color = "red";
    }
}

<h1 class="counter-display">(..)</h1>
<button class="counter-minus">-</button>
<button class="counter-plus">+</button>
<button class="reset">Reset</button>

answered Sep 28, 2022 at 8:32

Radheesh

171 silver badge3 bronze badges

1 Comment

Community Over a year ago

Your answer could be improved with additional supporting information. Please edit to add further details, such as citations or documentation, so that others can confirm that your answer is correct. You can find more information on how to write good answers in the help center.

Collectives™ on Stack Overflow

Simple Counter Javascript

5 Answers 5

Comments

Comments

Comments

Comments

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

5 Answers 5

Comments

Comments

Comments

Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related