0

I was trying to create a simple demo calculator and I got the output as NaN. I think it is something with the JS calling the input values. Please refer to the below code.

var num1 = document.getElementsByName(firstNoInput).value;
var num2 = document.getElementsByName(secondNoInput).value;
var ans;

function addition() {
  ans = num1 + num2;
  document.getElementById("screen").innerHTML = ans;
}

function subtraction() {
  ans = num1 - num2;
  document.getElementById("screen").innerHTML = ans;
}

function multiplication() {
  ans = num1 * num2;
  document.getElementById("screen").innerHTML = ans;
}

function division() {
  ans = num1 / num2;
  document.getElementById("screen").innerHTML = ans;
}
<!DOCTYPE html>
<html>

<head>
  <title>Calculator</title>
</head>

<body>
  <div>
    <header id="screen"></header>
    <table>
      <tbody>
        <tr>
          <td>First Number:</td>
          <td name="firstNoInput"><input type="text" /></td>
        </tr>
        <tr>
          <td>Second Number:</td>
          <td name="secondNoInput"><input type="text" /></td>
        </tr>
        <tr>
          <td><button type="submit" onclick="addition()">+</button></td>
          <td><button type="submit" onclick="subtraction()">-</button></td>
        </tr>
        <tr>
          <td><button type="submit" onclick="multiplication()">*</button></td>
          <td><button type="submit" onclick="division()">/</button></td>
        </tr>
      </tbody>
    </table>
  </div>

</body>

</html>

I even used the following code (convert values into strings), but it didn't work.

var num1 = parseInt(document.getElementsByName(firstNoInput).value);
var num2 = parseInt(document.getElementsByName(secondNoInput).value);

Can anyone help?

Rana
  • 2,500
  • 2
  • 7
  • 28
  • 2
    Your code shouldn't be working at all. `document.getElementsByName(firstNoInput).value` is wrong. It should be `document.getElementsByName('firstNoInput')[0].value` as it returns a _list_ of nodes, and you needed to quote the element name. The same for the the second selector. – Andy Jun 30 '22 at 21:02
  • Additionally your `name`s are on the wrong element. You want them on the inputs, not the `td` elements. That's why you're getting the error other than the other thing I mentioned. – Andy Jun 30 '22 at 21:05
  • @Andy No, it should be `document.querySelector("[name='firstNoINput']").value`. [`getElementsByTagName()` should not be used at all.](https://stackoverflow.com/questions/54952088/how-to-modify-style-to-html-elements-styled-externally-with-css-using-js/54952474#54952474) – Scott Marcus Jun 30 '22 at 21:05
  • You forgot to quote your selector @ScottMarcus – Andy Jun 30 '22 at 21:07
  • `name` is invalid on a `td` element. You should use `id` instead. – Scott Marcus Jun 30 '22 at 21:08
  • You'll be calculating on the old values of those inputs. – IT goldman Jun 30 '22 at 21:09

3 Answers3

1
var num1 = document.getElementsByName(firstNoInput).value;
var num2 = document.getElementsByName(secondNoInput).value;

is wrong. You should pass the parameters as strings. However, ignore this part, since the both variables that you define make only sense if your input fields already have values when the page gets loaded. Since you do not have these values, you will need a key listener that gets you the values whenever the user types.

Also, as described in the comments, getElementsByName() returns a NodeList and not an element.

let num1;
let num2;
document.querySelector('[name=firstNoInput]').addEventListener('keyup', (e) => {
  num1 = e.target.value;
});
document.querySelector('[name=secondNoInput]').addEventListener('keyup', (e) => {
  num2 = e.target.value;
});


function addition() {
  ans = Number(num1) + Number(num2);
  document.getElementById("screen").innerHTML = ans;
}

function subtraction() {
  ans = Number(num1) - Number(num2);
  document.getElementById("screen").innerHTML = ans;
}

function multiplication() {
  ans = Number(num1) * Number(num2);
  document.getElementById("screen").innerHTML = ans;
}

function division() {
  ans = Number(num1) / Number(num2);
  document.getElementById("screen").innerHTML = ans;
}
<header id="screen"></header>
<table>
  <tbody>
    <tr>
      <td>First Number:</td>
      <td name="firstNoInput"><input type="text" /></td>
    </tr>
    <tr>
      <td>Second Number:</td>
      <td name="secondNoInput"><input type="text" /></td>
    </tr>
    <tr>
      <td><button type="submit" onclick="addition()">+</button></td>
      <td><button type="submit" onclick="subtraction()">-</button></td>
    </tr>
    <tr>
      <td><button type="submit" onclick="multiplication()">*</button></td>
      <td><button type="submit" onclick="division()">/</button></td>
    </tr>
  </tbody>
</table>
Reza Saadati
  • 5,018
  • 4
  • 27
  • 64
  • [`getElementsByName('firstNoInput')[0]` is really bad](https://stackoverflow.com/questions/54952088/how-to-modify-style-to-html-elements-styled-externally-with-css-using-js/54952474#54952474) and `keyup` is not the correct event to use (what if the user wants to input a number greater than 9?). – Scott Marcus Jul 01 '22 at 13:37
  • @ScottMarcus related to `getElementsByName()` I have updated my answer. Related to `keyup` I don't get your point. What's wrong when the number is *10*? If you mean that value could be a non-nummeric-character, yes, the return will be *NaN* but then the value should be validated and that is not the point of this question. – Reza Saadati Jul 01 '22 at 15:33
  • @RezaSaadati With `keyup`, if someone enters a value that is more than a single keystroke, the event handler will fire multiple times unnecessarily. Instead, the `change` event should be used so that the event callback only fires when the field loses the focus and the value at that point has been altered. – Scott Marcus Jul 01 '22 at 15:39
  • @RezaSaadati Additionally, setting up those event handlers just to set the value of the variables is unnecessary in the first place. Just set up the `click` handler and, at that time, get the value of the inputs. – Scott Marcus Jul 01 '22 at 15:43
0

Your var num1 and num2 are being set only when you start the script. Since it looks like you would like to use those as variables for your calculations, we can use a getter function.

And you can use a setter function on the output.

// var num1 = document.getElementsByName(firstNoInput).value; 
// not the input! this is the td element.
// var num2 = document.getElementsByName(secondNoInput).value;

var num1 = { get value() { 
   return (document.getElementById("number1").value * 1); 
   // note multiply by 1 to make typeof number.
} };
var num2 = { get value() { 
   return (document.getElementById("number2").value * 1); 
   // note multiply by 1 to make typeof number.
} };

var ans = { set value(z) {        
   document.getElementById("answer").innerHTML = z} 
};



function addition() {
  ans.value = num1.value + num2.value;
  // note num1.value and num2.value must be typeof number.
}

function subtraction() {
  ans.value = num1.value - num2.value;
}

function multiplication() {
  ans.value = num1.value * num2.value;
}

function division() {
  ans.value = num1.value / num2.value;
}
<!DOCTYPE html>
<html>

<head>
  <title>Calculator</title>
</head>

<body>
  <div>
    <header id="screen"></header>
    <table>
      <tbody>
        <tr>
          <td>First Number:</td>
          <td name="firstNoInput"><input id="number1" type="text" value=""/></td>
        </tr>
        <tr>
          <td>Second Number:</td>
          <td name="secondNoInput"><input id="number2" type="text" value="" /></td>
        </tr>
        <tr>
          <td>Answer</td>
          <td id="answer"></td>
        </tr>  
        <tr>
          <td><button type="submit" onclick="addition()">+</button></td>
          <td><button type="submit" onclick="subtraction()">-</button></td>
        </tr>
        <tr>
          <td><button type="submit" onclick="multiplication()">*</button></td>
          <td><button type="submit" onclick="division()">/</button></td>
        </tr>
      </tbody>
    </table>
  </div>
  </body>
Wayne
  • 4,760
  • 1
  • 24
  • 24
0

There's a lot wrong with your approach (inline event attributes, invalid HTML, incorrect usage of table, etc.). The code is written like we did back in 1998. We now have standards and best practices.

See the answer below for comments:

<!DOCTYPE html>
<html>
  <head>
    <title>Calculator</title>
  </head>
  <body>
    <!-- Tables should not be used for layout -->
    <div>First Number: <input id="firstNoInput"></div>
    <div>Second Number: <input id="secondNoInput"></div>
    <div>
      <!-- By giving each button the same class, we can 
           differentiate them from all other elements easily.
           Also, you aren't submitting data anywhere, so you
           shouldn't be using type="submit". -->
      <button type="button" class="operator">+</button>
      <button type="button" class="operator">-</button>
      <button type="button" class="operator">*</button>
      <button type="button" class="operator">/</button>
    </div>
    <div id="result"></div>

    <script>
      // Use methods that return only a single element when
      // its only a single element you want. Notice here we're
      // only getting references to the elements. We'll get their
      // values later on when they are actually needed.
      const num1 = document.querySelector("#firstNoInput");
      const num2 = document.querySelector("#secondNoInput");
      const output = document.querySelector("#result");

      // Here, we'll set up just one click event on the document
      // and when any of the buttons get clicked, the event will
      // bubble up to the document and be handled by this:
      document.addEventListener("click", calc);

      // Since all your functions were essentially the same, except
      // for which kind of math they were doing, you can combine them
      function calc(event) {
        // Find out if the event was triggered by one of the buttons
        if(event.target.classList.contains("operator")){
         let answer = null;
          // Determine which button was clicked by the text within it
          switch (event.target.textContent) {
            case "+":
              // The input values are strings, but by prepending
              // a + to them, we can convert them to numbers. Notice that
              // it's here that we are getting the values of the elements
              answer = +num1.value + +num2.value;
              break;
            case "-":
              // The rest of the math operations will implicitly
              // convert the strings to numbers because the operators
              // can only work with numbers
              answer = num1.value - num2.value;
              break;  
            case "*":
              answer = num1.value * num2.value;
              break;   
            case "/":
              answer = num1.value / num2.value;
              break; 
          }
          // Update the DOM with the answer
          output.textContent = answer;
        }
      }
    </script>
  </body>
</html>
Scott Marcus
  • 64,069
  • 6
  • 49
  • 71