Better way of using JS function

Question

This code is a simple calculator. Enter 2 numbers and hit the required operation to get the output and there is nothing wrong in this code. But, I'm trying to find the possible ways of reducing the JavaScript code to the shortest.

<!DOCTYPE html>
<html lang="en">
<head>
    <meta charset="UTF-8">
    <meta name="viewport" content="width=device-width, initial-scale=1.0">
    <title>Document</title>
    <script>
        function add(){
            var a = parseInt(document.getElementById('no1').value);
            var b = parseInt(document.getElementById('no2').value);
            document.write(a+b);
        }
        function sub(){
            var a = document.getElementById('no1').value;
            var b = document.getElementById('no2').value;
            document.write(a-b);
        }
        function mult(){
            var a = document.getElementById('no1').value;
            var b = document.getElementById('no2').value;
            document.write(a*b);
        }
        function div(){
            var a = document.getElementById('no1').value;
            var b = document.getElementById('no2').value;
            document.write(a/b);
        }
    </script>
</head>
<body>
    <input type="text" id="no1">
    <input type="text" id="no2">
    <input type="button" value="ADD" onclick="add()">
    <input type="button" value="SUB" onclick="sub()"> 
    <input type="button" value="MULT" onclick="mult()">
    <input type="button" value="DIV" onclick="div()">
    </body>
</html>

CertainPerformance · Accepted Answer · 2020-02-14 03:44:46Z

4

You can use event delegation instead, on the body. On click, check to see if the clicked element is an input with a value - if so, look up the appropriate function to run on an object indexed by values (eg ADD, SUB).

You also might consider using type="number" encourage the user to only enter numeric values:

const fns = {
  ADD: (a, b) => a + b,
  SUB: (a, b) => a - b,
  MULT: (a, b) => a * b,
  DIV: (a, b) => a / b,
}
const [no1, no2] = ['no1', 'no2'].map(id => document.getElementById(id));
document.body.addEventListener('click', (e) => {
  if (!e.target.matches('input[value]')) {
    return;
  }
  const { value } = e.target;
  const fn = fns[value];
  console.log(
    fn(Number(no1.value), Number(no2.value))
  );
});

<input type="number" id="no1">
<input type="number" id="no2">
<input type="button" value="ADD">
<input type="button" value="SUB">
<input type="button" value="MULT">
<input type="button" value="DIV">

It could be made even shorter (by not selecting the elements ahead of time, or by relying on the ids being on the window, or by not storing temporary values in variables), but it'd be less efficient and/or less readable, so I wouldn't recommend it:

const fns = {
  ADD: (a, b) => a + b,
  SUB: (a, b) => a - b,
  MULT: (a, b) => a * b,
  DIV: (a, b) => a / b,
}
document.body.addEventListener('click', ({ target }) => {
  if (target.matches('input[value]')) {
    console.log(fns[target.value](Number(no1.value), Number(no2.value)));
  }
});

<input type="number" id="no1">
<input type="number" id="no2">
<input type="button" value="ADD">
<input type="button" value="SUB">
<input type="button" value="MULT">
<input type="button" value="DIV">

edited Feb 14, 2020 at 3:44

answered Feb 14, 2020 at 3:24

CertainPerformance

373k55 gold badges354 silver badges359 bronze badges

Sign up to request clarification or add additional context in comments.

3 Comments

zfrisch Over a year ago

Good answer! This is really well coded. I'm on mobile and haven't found a good way to test code, but if the number inputs are empty, I think it might return NaN? It might need a validation and default

CertainPerformance Over a year ago

The empty string, when cast to a number, becomes 0, so it's OK.

zfrisch Over a year ago

okay! Nicely done. I love seeing code structured to be dynamic. I'm going to see if I can rival it with a constructor or class using templating a bit later. +1

Aadit M Shah · Accepted Answer · 2020-02-14 04:07:44Z

Here is what I would do.

const inputX = document.querySelector("#x");
const inputY = document.querySelector("#y");
const output = document.querySelector("#z");
const select = document.querySelector("#op");

const operations = {
    add: (x, y) => x + y,
    sub: (x, y) => x - y,
    mul: (x, y) => x * y,
    div: (x, y) => x / y,
};

const render = (x, y, op) => {
    inputX.value = x;
    inputY.value = y;
    output.value = op(x, y);
    select.value = op.name;

    inputX.onchange = () => render(parseFloat(inputX.value, 10), y, op);
    inputY.onchange = () => render(x, parseFloat(inputY.value, 10), op);
    select.onchange = () => render(x, y, operations[select.value]);
};

render(0, 0, operations.add);

<input id="x" type="number"/>
<select id="op">
  <option value="add">+</option>
  <option value="sub">-</option>
  <option value="mul">*</option>
  <option value="div">/</option>
</select>
<input id="y" type="number"/> =
<output id="z"></output>

It's not as terse as CertainPerformance's answer. However, I feel that it's more understandable. In addition, it's reactive. Finally, although I haven't made use of event delegation here you can certainly use event delegation instead.

Collectives™ on Stack Overflow

Better way of using JS function

2 Answers 2

3 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

3 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related