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>

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!

• javascript
• html

Share Improve this question Follow edited Sep 28, 2022 at 8:03 Amadan 199k2323 gold badges252252 silver badges319319 bronze badges asked Sep 28, 2022 at 8:01 Cédric LeroyCédric Leroy 6744 silver badges99 bronze badges 2

• 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 Commented Sep 28, 2022 at 8:05
• As the error says;innerHTML is not a function- its a property....so counter.innerHTML = "Foo"; – Jamiec Commented Sep 28, 2022 at 8:05

Add a ment  | 

5 Answers 5

Sorted by: Reset to default 3

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>

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

1. 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.
2. 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.
3. 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.
4. 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.
5. 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>

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>

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>

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>