My html calculator display isn't reset after clicking an operator button

Question

function aClick(){
    if(a === undefined){
        numberButtons.forEach(button => {
            button.addEventListener("click", addNumber)
        });
        operatorButtons.forEach(button => {
            button.addEventListener('click', function(){
                a= parseInt(document.getElementsByClassName('up')[0].innerHTML)
                operator = button.id
                numberButtons.forEach(element => {
                    element.removeEventListener('click',addNumber)
                });
            })
        });
        document.getElementsByClassName('up')[0].innerHTML = "" //Line in question
        numberButtons.forEach(button => {
            button.addEventListener("click", addNumber)
        });
        resultButton.addEventListener('click', result)
    }
}

I'm trying to build a calculator display that will delete the content after an operator button has been pressed (+,-,*,/). However, currently, nothing changes after I press the button. Whatever number I presses earlier still remains. What is going on? FYI, I will be providing the code to addNumber() and result():

function addNumber(Event){
    document.getElementsByClassName('up')[0].innerHTML += Event.target.id
}

function result(){
    resultButton.addEventListener('click', function(){
        b = parseInt(document.getElementsByClassName('up')[0].innerHTML)
        document.getElementsByClassName('down')[0].innerHTML=operate(a,operator,b)
    })
}

Please click [edit] andpost a [mcve] of your attempt, noting input and expected output using the [\[<>\]](https://meta.stackoverflow.com/questions/358992/ive-been-told-to-create-a-runnable-example-with-stack-snippets-how-do-i-do) snippet editor. — mplungjan, Aug 22 '23 at 06:02

mplungjan · Answer 1 · 2023-08-31T10:22:29.303

Your code is very inefficient. You should never add an event handler in a click since it is added each time you click and I see you have then chosen to remove it again each time.

Here is a DRYer way using delegation

You can use a parser on the array instead of eval

Evaluating a string as a mathematical expression in JavaScript

let a;
let b;
let operator;

const functions = {
  add: (a, b) => a + b,
  subtract: (a, b) => a - b,
  multiply: (a, b) => a * b,
  divide: (a, b) => a / b
};

const operate = (a, operator, b) => {
  console.log(a, operator, b)
  switch (operator) {
    case "+":
      return functions.add(a, b)
    case "-":
      return functions.subtract(a, b)
    case "/":
      return functions.divide(a, b)
    case "*":
      return functions.multiply(a, b)
  }
};


let array = [];
const calculate = array => operate(+array[0], array[1], +array[2]); // no eval

document.getElementById("calc").addEventListener("click", (e) => {
  const tgt = e.target.closest("button"); // handle possible child elements like FontAwesome icons
  if (!tgt) return // clicked somewhere not a button
  if (tgt.matches(".operator")) {
    array.push(tgt.textContent);
    display.textContent = array.join(" ");
    // do something with operator and values if needed
    return;
  } else if (tgt.matches(".number")) {
    const number = tgt.textContent;
    if (array.length === 0) array.push(number)
    else if (array.length === 1) array[0] += number;
    else if (array.length === 2) array.push(number)
    else if (array.length === 3) array[2] += number;

    display.textContent = array.join(" ");
    // do something with number
    return;
  } else if (tgt.id === "equals") {
    if (array[1] === "/" && array[2] === "0") {
      display.textContent = "Cannot divide by 0";
      array = [];
      setTimeout(() => display.textContent = "",1000);
      return; 
    }
    display.textContent = calculate(array);
    array = [];    
  } else if (tgt.id === "clear") {
    display.textContent = "";
    array = [];
  }
})

div.button-layout div {
  padding: 0;
}

button {
  height: 25px;
  width: 25px;
  margin: 4px;
}

#display {
  border: 1px solid black;
  height: 25px;
  width: 140px;
  text-align: right;
}

<div id="calc">
  <div id="display">0</div>
  <div class="button-layout">
    <div>
      <button class="number">1</button>
      <button class="number">2</button>
      <button class="number">3</button>
      <button class="operator">+</button>
    </div>
    <div>
      <button class="number">4</button>
      <button class="number">5</button>
      <button class="number">6</button>
      <button class="operator">-</button>
    </div>
    <div>
      <button class="number">7</button>
      <button class="number">8</button>
      <button class="number">9</button>
      <button class="operator">*</button>
    </div>
    <div>
      <button id="clear">C</button>
      <button class="number">0</button>
      <button id="equals">=</button>
      <button class="operator">/</button>
    </div>
  </div>
</div>

Any ways I can go about this without using eval()? I will not use it due to the security risks. — Carl Warren, Aug 22 '23 at 06:51
Eval is not a security risk if you decide what can be eval'ed. In this case the user can only click your buttons, not enter anything else. Anyway, you need a parser if you want to not use eval: https://stackoverflow.com/questions/2276021/evaluating-a-string-as-a-mathematical-expression-in-javascript — mplungjan, Aug 22 '23 at 07:00
@CarlWarren I made a version that uses your calculate instead of eval. — mplungjan, Aug 22 '23 at 13:00

score -1 · Accepted Answer · answered Aug 22 '23 at 05:01

To make sure that the display area content is cleared after an operator button is clicked, you should place the line document.getElementsByClassName('up')[0].innerHTML = "" inside the event listener for the operator buttons. It should be placed within the callback function that is executed when an operator button is clicked.

function aClick() {
    if (a === undefined) {
        numberButtons.forEach(button => {
            button.addEventListener("click", addNumber);
        });
        operatorButtons.forEach(button => {
            button.addEventListener('click', function () {
                a = parseInt(document.getElementsByClassName('up')[0].innerHTML);
                operator = button.id;
                numberButtons.forEach(element => {
                    element.removeEventListener('click', addNumber);
                });

                // Move this line here
                document.getElementsByClassName('up')[0].innerHTML = "";
            });
        });
        
        // Move this line outside of the operatorButtons loop
        numberButtons.forEach(button => {
            button.addEventListener("click", addNumber);
        });

        resultButton.addEventListener('click', result);
    }
}

Thanks for the insight. Do you have any idea why the line doesn't do anything when I moved it out of the eventListeners for the operatorButtons? — Carl Warren, Aug 22 '23 at 06:48

My html calculator display isn't reset after clicking an operator button

2 Answers2

Linked