0

I'm trying to create a simple javascript calculator with only 4 operations (+, -, * and /). I've created an HTML file with 4 fields:

  • Number 1
  • Number 2
  • Operation
  • Answer

I have also created a script file with 4 functions (one for each operation) but I can't seem to work out where/how to add my an if statement so the appropriate function is called based on the operation selection.

function sumValues() {
  var num1, num2, ans;

  num1 = Number(document.formcalc.num1.value);
  num2 = Number(document.formcalc.num2.value);
  ans = num1 + num2;
  document.formcalc.answer.value = ans;
}

function subValues() {
  var num1, num2, answer;

  num1 = Number(document.formcalc.num1.value);
  num2 = Number(document.formcalc.num2.value);
  ans = num1 - num2;
  document.formcalc.answer.value = ans;
}

function multiValue() {
  var num1, num2, ans;

  num1 = Number(document.formcalc.num1.value);
  num2 = Number(document.formcalc.num2.value);
  ans = num1 * num2;
  document.formcalc.answer.value = ans;
}

function divValue() {
  var num1, num2, ans;

  num1 = Number(document.formcalc.num1.value);
  num2 = Number(document.formcalc.num2.value);
  ans = num1 / num2;
  document.formcalc.answer.value = ans;
}
<h1>My Calculator</h1>
<form name="formcalc">
  Number 1: <input type="text" name="num1"><br>
  Number 2: <input type="text" name="num2"><br>
  Operation(+,-,*,/): <input type="text" name="selection"><br>
  Answer: <input type="text" name="answer"><br>
  <input type="button" value="calculate" onclick="divValue()">
</form>
MTCoster
  • 5,868
  • 3
  • 28
  • 49
shaz
  • 5
  • 2
  • Perhaps you could create a new function, eg `onCalculateClick()` or similar, which reads in the operation and decides which calculation function to call? – Alex McMillan Feb 09 '19 at 22:02

4 Answers4

2

Unless this example is explicitly to help you learn about how to use functions, there's really no need for an entire method for each operation.

Look at how much of your code is repeated, almost identically - such as retrieving the values from the DOM and setting the answer. These pieces of code should really only appear once in your entire program.

The only part which varies actually, is the operation itself - which can be branched for in a simple switch statement.

function onCalculateClick() {
  const num1 = Number(document.getElementById('num1').value);
  const num2 = Number(document.getElementById('num2').value);
  const operation = document.getElementById('operation').value;

  let ans = "Invalid Operation";

  switch (operation) {
    case "+": ans = num1 + num2; break;
    case "-": ans = num1 - num2; break;
    case "*": ans = num1 * num2; break;
    case "/": ans = num1 / num2; break;
  }
  
  document.getElementById('answer').value = ans;
}
<h1>My Calculator</h1>
<form name="formcalc">
  Number 1: <input type="text" id="num1"><br />
  Number 2: <input type="text" id="num2"><br />
  Operation(+,-,*,/): <input type="text" id="operation"><br />
  Answer: <input type="text" id="answer"><br />
  <input type="button" value="calculate" onclick="onCalculateClick()">
</form>
Alex McMillan
  • 17,096
  • 12
  • 55
  • 88
0

I'll try and give some pointers without completing your school assignment for you haha.

What you are missing is to hook up your "calculate" button with a new function (you need to write it), that based on "selection" operator will "switch" it to the correct js function (divValue, multiValue) etc

Good luck

silvio
  • 5,651
  • 2
  • 17
  • 14
0

You’ll need to create a fifth function (let’s call it calculate()) which is called when the button is clicked, and dispatches to the correct operation function. Here’s what that would look like using if statements (as per your question):

function calculate() {
  var op = document.selection.value;

  if (op === '+') {
    sumValues();
  } else if (op === '-') {
    subValues();
  } else if (op === '*') {
    multiValue();
  } else if (op === '/') {
    divValue();
  } else {
    console.log('Invalid operation:', op);
  }
}

However, there is a better construct to use when you come across layered if statements like this; a switch:

function calculate() {
  var op = document.selection.value;

  switch (op) {
    case '+':
      sumValues();
      break;

    case '-':
      subValues();
      break;

    case '*':
      multiValue();
      break;

    case '/':
      divValue();
      break;

    default:
      console.log('Invalid operation:', op);
  }
}

Or more compactly:

function calculate() {
  var op = document.selection.value;

  switch (op) {
    case '+': sumValues(); break;
    case '-': subValues(); break;
    case '*': multiValue(); break;
    case '/': divValue(); break;
    default: console.log('Invalid operation:', op);
  }
}

Adding this to your existing code gives us:

function sumValues() {
  var num1, num2, ans;

  num1 = Number(document.formcalc.num1.value);
  num2 = Number(document.formcalc.num2.value);
  ans = num1 + num2;
  document.formcalc.answer.value = ans;
}

function subValues() {
  var num1, num2, answer;

  num1 = Number(document.formcalc.num1.value);
  num2 = Number(document.formcalc.num2.value);
  ans = num1 - num2;
  document.formcalc.answer.value = ans;
}

function multiValue() {
  var num1, num2, ans;

  num1 = Number(document.formcalc.num1.value);
  num2 = Number(document.formcalc.num2.value);
  ans = num1 * num2;
  document.formcalc.answer.value = ans;
}

function divValue() {
  var num1, num2, ans;

  num1 = Number(document.formcalc.num1.value);
  num2 = Number(document.formcalc.num2.value);
  ans = num1 / num2;
  document.formcalc.answer.value = ans;
}

function calculate() {
  var op = document.selection.value;

  switch (op) {
    case '+': sumValues(); break;
    case '-': subValues(); break;
    case '*': multiValue(); break;
    case '/': divValue(); break;
    default: console.log('Invalid operation:', op);
  }
}
<h1>My Calculator</h1>
<form name="formcalc">
  Number 1: <input type="text" name="num1"><br>
  Number 2: <input type="text" name="num2"><br>
  Operation(+,-,*,/): <input type="text" name="selection"><br>
  Answer: <input type="text" name="answer"><br>
  <input type="button" value="calculate" onclick="calculate()">
</form>

A few notes on your code:

Avoid using the name attribute

It’s considered deprecated at this point. Instead, opt for the id attribute:

<input type="text" id="operation">

function calculate() {
  var op = document.getElementById('operation').value;
  ...
}

Keep your function names consistent

You’ve got two functions with Values in the name and two with Value. Pick one and stick with it, it’ll save you a headache later when you use the wrong one by mistake.

Avoid duplicate code

The majority of each xxxValues() function is identical to the rest. You can simplify your code by refactoring to lift repeated code into a common function:

function calculate() {
  var num1 = Number(document.getElementById('num1').value),
      num2 = Number(document.getElementById('num2').value),
      op = document.getElementById('operation').value;
      ans;

  switch (op) {
    case '+': ans = sumValues(num1, num2); break;
    case '-': ans = subValues(num1, num2); break;
    case '*': ans = multiValues(num1, num2); break;
    case '/': ans = divValues(num1, num2); break;
    default: console.log('Invalid operation:', op);
  }

  document.getElementById('answer').value = ans;
}

Now your xxxValues() functions can just be a single line, for example:

function sumValues(num1, num2) {
  return num1 + num2;
}

Although with such simple functions, there’s really no need for them to be separate functions at all at this point, so we can inline them:

function calculate() {
  var num1 = Number(document.getElementById('num1').value),
      num2 = Number(document.getElementById('num2').value),
      op = document.getElementById('operation').value;
      ans;

  switch (op) {
    case '+': ans = num1 + num2; break;
    case '-': ans = num1 - num2; break;
    case '*': ans = num1 * num2; break;
    case '/': ans = num1 / num2; break;
    default: console.log('Invalid operation:', op);
  }

  document.getElementById('answer').value = ans;
}

Here’s how the code looks now:

function calculate() {
  var num1 = Number(document.getElementById('num1').value),
      num2 = Number(document.getElementById('num2').value),
      op = document.getElementById('operation').value;
      ans;

  switch (op) {
    case '+': ans = num1 + num2; break;
    case '-': ans = num1 - num2; break;
    case '*': ans = num1 * num2; break;
    case '/': ans = num1 / num2; break;
    default: console.log('Invalid operation:', op);
  }

  document.getElementById('answer').value = ans;
}
<h1>My Calculator</h1>
<form id="formcalc">
  Number 1: <input type="text" id="num1"><br>
  Number 2: <input type="text" id="num2"><br>
  Operation(+,-,*,/): <input type="text" id="operation"><br>
  Answer: <input type="text" id="answer"><br>
  <input type="button" value="calculate" onclick="calculate()">
</form>

And finally, if you feel like taking things way too far

const calculatorFuncs = {
  '+': (n1, n2) => n1 + n2,
  '-': (n1, n2) => n1 - n2,
  '*': (n1, n2) => n1 * n2,
  '/': (n1, n2) => n1 / n2
}

function calculate() {
  const op = document.getElementById('operation').value;

  if (!'+-*/'.split('').includes(op)) {
    console.log('Invalid operation:', op);
    return;
  }

  document.getElementById('answer').value = calculatorFuncs[op](...([...Array(2).keys()].map(n => document.getElementById(`num${++n}`).value)));
}

I’ll leave it as an exercise to OP to figure out what’s going on here ;)

MTCoster
  • 5,868
  • 3
  • 28
  • 49
0

You can just add another function which will select the operation to be called.

Example:

function selectOperation(){

    var selection = document.formcalc.selection.value;
    switch(selection[0]) {
      case '+':
        sumValues();
        break;
      case '-':
        subValues();
        break;
      case '*':
        multiValue();
        break;
      case '/':
        divValue();
        break;
      default:
        // Prompt something if input is wrong
    } 
}

And then in the input you call this function:

<input type="button" value="calculate" onclick="selectOperation()">

I just tried here and works perfectly! Let me know

perellorodrigo
  • 536
  • 5
  • 11