How do I replace the dangerous eval()-method with something safer? Do I have to go the long way with a switch case?

Question

I'm practicing Javascript and started out with a calculator-code with lots of repetition. I simplified it quite a bit, but I know the eval()-method isn't... hehe... "recommended".


let currentResult = 0;

function calculate(method){
  const calculation = `${currentResult} ${method} ${userInput.value}`;
  currentResult = eval(calculation);
  outputResult(currentResult, calculation);
}
addBtn.addEventListener('click', ()=> calculate('+'));
subtractBtn.addEventListener('click', ()=> calculate('-'));
multiplyBtn.addEventListener('click', ()=> calculate('*'));
divideBtn.addEventListener('click', ()=> calculate('/'));

`if (method === '+') calculation = currentResult + Number(userInput.value);` and etc... — Nir Alfasi, May 21 '20 at 10:00
@alfasin it's nice but require an entire line for each case. I'm trying to avoid writing a function for each and if possible also switch case mapping to an object with operators in strings in keys and calculations in value. — Anders Clark, May 21 '20 at 10:43
True, there's a trade-off between the two approaches: if you don't want to "know" which operation the user wants you to take, you can keep using eval, which is risky. On the other hand, you need to be more verbose and parse the input in order to not use eval but still be able to apply the correct method. I prefer the latter option because it's easier to read & understand, debug, extend (say you now want to support more operations like root square, power, factorial etc) and most important: it comes with input validation "out of the box" which is always a good practice! — Nir Alfasi, May 21 '20 at 10:51

score 1 · Answer 1 · answered May 21 '20 at 10:03

You could take an object with the function for each operator.

const
    operators = {
        '+': (a, b) => a + b,
        '-': (a, b) => a - b,
        '*': (a, b) => a * b,
        '/': (a, b) => a / b
    };

let currentResult = 0;

function calculate(method) {
    if (sign in operators) {
        currentResult = operators[sign](currentResult, +userInput.value);
    }
    outputResult(currentResult, calculation);
}

addBtn.addEventListener('click', () => calculate('+'));
subtractBtn.addEventListener('click', () => calculate('-'));
multiplyBtn.addEventListener('click', () => calculate('*'));
divideBtn.addEventListener('click', () => calculate('/'));

Thanks! That is my solution also. Though I'm still looking for a solution that only requires one line instead of five. — Anders Clark, May 21 '20 at 10:43

How do I replace the dangerous eval()-method with something safer? Do I have to go the long way with a switch case?

1 Answers1