Basic arithmetic calculator in JavaScript

Task is basically:

"Write a function which takes two number as the first two parameter. The third parameter is one of the arithmetic operators (+ , - , * , /). Execute the arithmetic operation (number1 operator number2) and return the result of it."

Here's the first idea I've had:



function calculate(num1, num2, operator) {
num1 = parseFloat(num1);

if (isNaN(num1)) {
throw Error('First parameter invalid. Must be a number.');
}

num2 = parseFloat(num2);

if (isNaN(num2)) {
throw Error('Second parameter invalid. Must be a number.');
}

operator = operator.trim();

if (!/^\+$|^\-$|^\*$|^\/$/.test(operator)) {
throw Error('Third parameter invalid. Valid are + | - | * | /');
}

return eval(num1 + operator + num2);
}



Then I've had another idea for the implementation which uses the ES6-syntax:



function calculate2(num1, num2, operator) {
num1 = parseFloat(num1);

if (isNaN(num1)) {
throw Error('First parameter invalid. Must be a number.');
}

num2 = parseFloat(num2);

if (isNaN(num2)) {
throw Error('Second parameter invalid. Must be a number.');
}

operator = operator.trim();

if (!/^\+$|^\-$|^\*$|^\/$/.test(operator)) {
throw Error('Third parameter invalid. Valid are + | - | * | /');
}

var calculations = {
'+' : (numb1, numb2) => numb1 + numb2,
'-' : (numb1, numb2) => numb1 - numb2,
'' : (numb1, numb2) => numb1 numb2,
'/' : (numb1, numb2) => numb1 / numb2
}

return calculationsoperator;
}



I still like the eval-version a bit better.

Can't see any harm which eval should create here because it's controlled via regular expression what is passed until that point.

What's your opinion?

Which implementation is better one?

Looking forward reading your answers.

I would go with the second one. Not because eval here is insecure -- you're validating the arguments -- but because it's overkill. eval is designed to parse and run any JavaScript code, so it is slow (compared to normal JavaScript code); engines do a lot of optimisation before running code, but they can't optimise if the code is in a dynamic string which changes at runtime. It probably won't maker much difference for you here, but if you're building a larger app, blocking JavaScript can be noticeable.

Here are the changes I would make to calculate2:

'use strict'; // Strict mode === good

function calculate2(num1, num2, operator) {
  // Number.parseFloat is new in ES6
  // (it's identical to window.parseFloat, so you can use either)
  num1 = Number.parseFloat(num1);
  num2 = Number.parseFloat(num2);
  operator = operator.trim();

  // Avoid `var`. ES6 brings `let` and `const`, which:
  //   - are block scoped
  //   - work with destructuring
  //   - prevent redeclaration in the same scope
  //   - aren't hoisted
  // Note that `const` doesn't freeze object properties
  // If you need frozen properties, use Object.freeze
  const calculations = {
    // Shorter names, it's clear from the context what they do.
    '+' : (a, b) => a + b,
    '-' : (a, b) => a - b,
    '*' : (a, b) => a * b,
    '/' : (a, b) => a / b, // You can use trailing commas in object and array literals if you want.
  }; // If you use semicolons, use them all the time.

  // Detailed error messages.
  // 
  // Number.isNan is also new in ES6,
  // however it's not quite the same as window.isNan.
  // Here it won't make a difference which you use.
  // 
  // `throw new Error` rather than `throw Error`
  // as you're using the Error constructor.
  if (Number.isNan(num1)) throw new Error(
    `Expected first argument to calculate2 to be a String valid for Number.parseFloat but got ${num1}.`);
  if (Number.isNan(num2)) throw new Error(
    `Expected second argument to calculate2 to be a String valid for Number.parseFloat but got ${num2}.`);

  // Don't bother maintaining a separate regex of valid operators
  // Just check the calculations object
  if (!calculations[operator]) throw new Error(
    `Expected third argument to calculate2 to be one of ${Object.keys(calculations).join(' ')} but got ${operator}.`);

  return calculations[operator](num1, num2);
}

StackExchange Code Review Q#133088, answer score: 6