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 list 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);
}
