Why does the "eval" function return "undefined"?

Question

I'm beginner in coding, still in learning process, and I stuck on some problem creating a simple Calculator app as part of my practice in JS. I have problem that when I make some prompt in the calculator screen as 3+3, than when I press equal I get the result 3+3undefined.

First I checked if I selected the variables correctly, then if I assigned add Listener correctly, then I tried to use math expression evaluation, but all without success. Everything I've tried always either throws Error or undefined.

(function() {
  let screen = document.querySelector('#display');
  let buttons = document.querySelectorAll('.btn');
  let clear = document.querySelector('#clear');
  let equal = document.querySelector('#equals');

  buttons.forEach(function(button) {
    button.addEventListener('click', function(e) {
      let value = e.target.dataset.num;
      screen.value += value;
    });
  });

  equal.addEventListener('click', function(e) {
    if (screen.value === '') {
      screen.value = "";
    } else {
      let answer = eval(screen.value); /* every check says that there is a code error in this line.*/
      screen.value = answer;
    }
  });

  clear.addEventListener('click', function(e) {
    screen.value = "";
  });
})();

<div class="calculator">
  <form>
    <input type="text" class="screen" id="display">
  </form>

  <div class="buttons">
    <button type="button" class="btn btn-yellow" id="multiply" data-num="*">*</button>
    <button type="button" class="btn btn-yellow" id="divide" data-num="/">/</button>
    <button type="button" class="btn btn-yellow" id="subtract" data-num="-">-</button>
    <button type="button" class="btn btn-yellow" id="add" data-num="+">+</button>
    <button type="button" class="btn" id="nine" data-num="9">9</button>
    <button type="button" class="btn" id="eight" data-num="8">8</button>
    <button type="button" class="btn" id="seven" data-num="7">7</button>
    <button type="button" class="btn" id="decimal" data-num=".">.</button>
    <button type="button" class="btn" id="six" data-num="6">6</button>
    <button type="button" class="btn" id="five" data-num="5">5</button>
    <button type="button" class="btn" id="four" data-num="4">4</button>
    <button type="button" class="btn btn-red" id="clear">C</button>
    <button type="button" class="btn" id="three" data-num="3">3</button>
    <button type="button" class="btn" id="two" data-num="2">2</button>
    <button type="button" class="btn" id="one" data-num="1">1</button>
    <button type="button" class="btn btn-green" id="equals">=</button>
    <button type="button" class="btn" id="zero" data-num="0">0</button>
  </div>
</div>

<script src="./script.js"></script>

Answer 1

You need to test you get an actual value from the button's data attribute.

It makes it easier if you delegate and you can simply use the textContent of the buttons. I have removed the data attributes completely

Added a sanitiser

(function() {
  // strip anything other than digits, (), -+/* and .
  const sanitizer = str => str.replace(/[^-()\d/*+.]/g, '');
  const screen = document.getElementById("display");
  document.querySelector(".buttons").addEventListener('click', function(e) {
    const tgt = e.target; // what was clicked
    if (tgt.matches("#equals")) {
      if (screen.value !== "") { // simpler test too
        try { 
          let answer = eval(sanitizer(screen.value)); 
        }
        catch (e) {
          console.log(e.message);
          return;
        }
        screen.value = answer;
      }
    } else if (tgt.matches("#clear")) {
      screen.value = "";
    } else { 
      let value = tgt.textContent;
      if (value) screen.value += value;
    }
  });
})();

<link rel="stylesheet" href="https://cdn.jsdelivr.net/npm/bootstrap@5.2.3/dist/css/bootstrap.min.css" />

<div class="calculator">
  <form>
    <input type="text" class="screen" id="display">
  </form>

  <div class="buttons">
    <button type="button" class="btn btn-yellow" id="multiply">*</button>
    <button type="button" class="btn btn-yellow" id="divide">/</button>
    <button type="button" class="btn btn-yellow" id="subtract">-</button>
    <button type="button" class="btn btn-yellow" id="add">+</button><br/>
    <button type="button" class="btn">9</button>
    <button type="button" class="btn">8</button>
    <button type="button" class="btn">7</button>
    <button type="button" class="btn">.</button><br/>
    <button type="button" class="btn">6</button>
    <button type="button" class="btn">5</button>
    <button type="button" class="btn">4</button>
    <button type="button" class="btn btn-red" id="clear">C</button><br/>
    <button type="button" class="btn">3</button>
    <button type="button" class="btn">2</button>
    <button type="button" class="btn">1</button>
    <button type="button" class="btn btn-green" id="equals">=</button><br/>
    <button type="button" class="btn">0</button>
  </div>
</div>

<script src="./script.js"></script>

Answer 2

Your equal button is firing two events.

One for button.addEventListener and the other equal.addEventListener. When the equal button is clicked it shouldn't run the button code because data-num does not exist. This is why undefined is being raised in your eval.

A cleaner solution is to check if the data attribute data-num exists instead of adding data-num to the equal button. Helps avoid adding unnecessary HTML.

  buttons.forEach(function(button) {
    button.addEventListener('click', function(e) {
      if (e.target.dataset.hasOwnProperty('num')) {
        let value = e.target.dataset.num;
        screen.value += value;
      }
    });
  });

Answer 3

this code fix your problem

equals button dose not have data-num property trigger undefine

buttons.forEach(function(button) {
    button.addEventListener('click', function(e) {
      
      if(e.target.id !="equals"){
        let value = e.target.dataset.num;
        screen.value += value;
       } 
   
    });
});

Answer 4

On this line in your HTML:

<button type="button" class="btn btn-green" id="equals">=</button>

Add data-num=""

<button type="button" class="btn btn-green" data-num="" id="equals">=</button>

Explanation: You always add the data-num value to the input field in your JS, if it is undefined you will get a value such as "9+9undefined", and that can't be handled by eval. If you add an emptry string the same value will be "9+9".

Edit (after problem solved)

As you said, you are still learning, and I think your implementation and your logic looks just fine.

So this is only a sidenote: If you want to you can let JS do more of the work of 'drawing' the calculator and also listen to events a little bit differently, by listening to clicks on the body and then filter out if any of the clicks actually hit a certain button (this is called delegated event handling). There are so many flavors of how you solve something like a simple calculator in JS and that's what's makes programming fun. Here's how I would have done it, if in a hurry :)

Also: Make sure that 0.1 + 0.2 returns 0.3, this may take some work. See: Is floating point math broken?

document.body.innerHTML = `
    <div class="calculator">
      <input type="text" placeholder="0" readonly>`
  + '789/456*123-C0.+()^='.split('')
    .map(x => `<div class="btn `
      + `${isNaN(x) && x !== '.' ? 'ctrl' : ''}">`
      + `${x}</div>`).join('') + '</div>';

document.body.addEventListener('click', e => {
  let btn = e.target.closest('.btn');
  if (!btn) { return; }
  let input = document.querySelector('input');
  let val = btn.innerText;
  val === 'C' && (input.value = '');
  val === '=' && (() => {
    try {
      input.value = eval(input.value.replaceAll('^', '**'))
        .toFixed(10).replace(/(\.[1-9]*)0{1,}$/g, '$1')
        .replace(/\.$/g, '');
    }
    catch (e) { input.value = ''; }
  })();
  !'C='.includes(val) && (input.value += val);
});

.calculator {
  border-radius: 0.5vw;
  background-color: rgb(41, 111, 164);
  padding: 10px;
  display: inline-block;
}

.calculator::after {
  content: "";
  display: table;
  clear: both;
}

.btn {
  width: 3vw;
  height: 3vw;
  background-color: black;
  margin: 1px;
  color: white;
  float: left;
  font-size: 2vw;
  font-family: Verdana;
  padding: 0.8vw 0.5vw 0.2vw;
  text-align: center;
  cursor: pointer;
}

.btn:nth-child(4n+2) {
  clear: both;
}

.btn.ctrl {
  background-color: #333;
}

input {
  font-family: Verdana;
  font-size: 2vw;
  width: 16vw;
  height: 3vw;
  margin-bottom: 1vw;
  display: block;
  text-align: right;
}

<!DOCTYPE html>
<html lang="en">
<head>
  <meta charset="UTF-8">
  <meta http-equiv="X-UA-Compatible" content="IE=edge">
  <title>Calculator</title>
</head>
<body>
</body>
</html>