Javascript Calculator eval() Function

In the Javascript Calculator project, am I taking a shortcut or cheating using the eval() function? Also, on the MDN eval() function webpage, it says use of the eval() function is not recommended due to security purpose, although I imagine that security is out of scope for this project. Any help would be appreciated. Thanks.

It’s fine to use eval().

If you can avoid using the eval() function in your code. Not because of any security reasons but just to see if you can implement the code on your own. This will help you be a better programmer.
Try imitating the use of eval() with some custom function.

Ok. It took me a while, but I wrote my own evaluate function. Let me know what you think. Like if I took the long way. Any tips would be appreciated.

function evaluate(str) {
  function splitStringArithmetic(str) {
    var index = 0;
    var splitArray = str.split("").reduce((arr, v, i) => {
      if (isNaN(parseInt(v))) {
        arr.push(str.slice(index, i));
        arr.push(v);
        index = i + 1;
      }
      return arr;
    }, []);
    splitArray.push(str.slice(index));
    return splitArray;
  }

  function findMultIndex(arr) {
    return arr.findIndex(i => i == "*");
  }

  function findDivIndex(arr) {
    return arr.findIndex(i => i == "/");
  }

  function findAddIndex(arr) {
    return arr.findIndex(i => i == "+");
  }

  function findSubIndex(arr) {
    return arr.findIndex(i => i == "-");
  }

  function multiply(arr) {
    var index = findMultIndex(arr);
    arr[index] = parseInt(arr[index - 1]) * parseInt(arr[index + 1]);
    return arr.filter((c, i) => {
      return i !== index - 1 && i !== index + 1;
    });
  }

  function divide(arr) {
    var index = findDivIndex(arr);
    arr[index] = parseInt(arr[index - 1]) / parseInt(arr[index + 1]);
    return arr.filter((c, i) => {
      return i !== index - 1 && i !== index + 1;
    });
  }

  function add(arr) {
    var index = findAddIndex(arr);
    arr[index] = parseInt(arr[index - 1]) + parseInt(arr[index + 1]);
    return arr.filter((c, i) => {
      return i !== index - 1 && i !== index + 1;
    });
  }

  function subtract(arr) {
    var index = findSubIndex(arr);
    arr[index] = parseInt(arr[index - 1]) - parseInt(arr[index + 1]);
    return arr.filter((c, i) => {
      return i !== index - 1 && i !== index + 1;
    });
  }

  function containsMultOrDiv(arr) {
    return arr.some(i => i === "*" || i === "/");
  }

  function containsAddOrSub(arr) {
    return arr.some(i => i === "+" || i === "-");
  }

  function simplify(arr) {
    while (containsMultOrDiv(arr)) {
      if (arr.includes("*")) {
        if (arr.includes("/")) {
          if (findDivIndex(arr) < findMultIndex(arr)) {
            arr = divide(arr);
          } else {
            arr = multiply(arr);
          }
        } else {
          arr = multiply(arr);
        }
      } else {
        arr = divide(arr);
      }
    }
    while (containsAddOrSub(arr)) {
      if (arr.includes("+")) {
        if (arr.includes("-")) {
          if (findSubIndex(arr) < findAddIndex(arr)) {
            arr = subtract(arr);
          } else {
            arr = add(arr);
          }
        } else {
          arr = add(arr);
        }
      } else {
        arr = subtract(arr);
      }
    }
    return arr;
  }

  var arithmeticArray = splitStringArithmetic(str);

  return simplify(arithmeticArray);
}
var str = "10-9+6*3/2";
console.log(evaluate(str));

The four functions are 99% identical except for the first line inside the function. See if you can rewrite these to remove the duplicate code. Whenever you have duplicate code, you have an opportunity to put it into a reusable function.

You have the same opportunity to make your code DRY (do not repeat yourself) with the following four functions:

  function findMultIndex(arr) {
    return arr.findIndex(i => i == "*");
  }

  function findDivIndex(arr) {
    return arr.findIndex(i => i == "/");
  }

  function findAddIndex(arr) {
    return arr.findIndex(i => i == "+");
  }

  function findSubIndex(arr) {
    return arr.findIndex(i => i == "-");
  }

Think back to the challenge named Using Objects for Lookups and see if you can replace these four functions above by using an Object.

2 Likes

Wouldn’t really agree with that. Some things are built in JS to help you because its simple and we all know how to do it. I also agree too, that you should know how to do stuff like this.

1 Like

Thanks for the advice! I added some more arguments to certain functions to combine them and this is what I have now. I think I reduced the code by 40 lines by combining and now It’s more clear.

function evaluate(str) {
  function splitStringArithmetic(str) {
    var index = 0;
    var splitArray = str.split("").reduce((arr, v, i) => {
      if (isNaN(parseInt(v))) {
        arr.push(str.slice(index, i));
        arr.push(v);
        index = i + 1;
      }
      return arr;
    }, []);
    splitArray.push(str.slice(index));
    return splitArray;
  }

  function findOperator(arr, o) {
    return arr.findIndex(i => i == o);
  }

  function arithmetic(o, a, b) {
    var arithmeticObject = {
      "*": a * b,
      "/": a / b,
      "+": a + b,
      "-": a - b
    };
    return arithmeticObject[o];
  }

  function compute(arr, o) {
    var index = findOperator(arr, o);
    arr[index] = arithmetic(o, arr[index - 1], arr[index + 1]);
    return arr.filter((c, i) => {
      return i !== index - 1 && i !== index + 1;
    });
  }
  function containsOperators(arr) {
    return arr.some(i => i === "*" || i === "/" || i === "+" || i === "-");
  }

  function simplify(arr) {
    while (containsOperators(arr)) {
      if (arr.includes("*") && arr.includes("/")) {
        if (findOperator(arr, "/") < findOperator(arr, "*")) {
          arr = compute(arr, "/");
        } else {
          arr = compute(arr, "*");
        }
      } else if (arr.includes("*")) {
        arr = compute(arr, "*");
      } else if (arr.includes("/")) {
        arr = compute(arr, "/");
      } else if (arr.includes("+") && arr.includes("-")) {
        if (findOperator(arr, "-") < findOperator(arr, "+")) {
          arr = compute(arr, "-");
        } else {
          arr = compute(arr, "+");
        }
      } else if (arr.includes("+")) {
        arr = compute(arr, "+");
      } else {
        arr = compute(arr, "-");
      }
    }
    return arr;
  }

  var arithmeticArray = splitStringArithmetic(str);
  return parseInt(simplify(arithmeticArray));
}
var str = "10*10/10+5*10-10";
console.log(evaluate(str));

Great, comeback 7 months later and be completely confused, or spend a unnecessary time reading comments about the function that is already a simple keyword in JS.

Creating a function to do math is medium to advanced, that is why its a built in keyword. Math is all something we know how to do already, so its a waste of time doing this.

use of eval is not recommended because eval can evaluate any JavaScript code written inside it.So,due to security reasons using eval is generally avoided.It would be better if you create your own eval.

I’d normally say not to use eval but since you’re doing it on the client-side there’s 0 risk, just remember to never use it on a server-side code or anywhere else but this challenge.

This calculator challenge is rather simple and you can either make a tokenized calculator with the shunting yard algorithm and an RPN evaluator algorithm for inputs that have parenthesis and such. Or just a simple “keep track of the key presses and save numbers in 2 variables so that 1 operator can act on them” type of deal.

It all depends on what you see as the spirit of the challenge: if it’s just to design an interactive calculator web UI, then you may as well build up an expression string and use eval(). If you see it as the larger challenge of modeling a state machine, then eval is absolutely the last thing you want. In the end, it’s a toy project: have fun with it (that’s my theme of the day and the year) but move on when you’re satisfied with it, and if that means using eval, so be it.

Okay eval function is not recommended because anyone can inject some malicious code and eval will execute it. Can’t we use eval function after validating the input string. We can ensure that the input string should only contain numbers and some symbols of mathematical operations. Will it be okay to do?

Can’t we use eval function after validating the input string

Technically yes, but I’d mention a few things

First, the user input is running on the user’s computer anyway, so presumably they know what they’re doing as risk goes. They could just type a javascript: URL in their browser and get the same thing after all. If you constrain the input so they can only type numbers and operators, you’re fine. That’s validation in a way, but done by constraining the possible input rather than checking it after the fact.

The second is that “validate then run with full power” as a design philosophy just isn’t a very good one, and has usually led to massive security holes. Sometimes the validation is buggy or fails to notice a corner case, or sometimes one just forgets to run the validator entirely. I’ve done both as recently as last week (crashing the web page was the worst result, thankfully).

So instead of “validate and run”, you “parse and interpret”. The difference is that instead of validating a string and handing back the same string, you hand back something completely different. Such as breaking the string into tokens represented as an array of objects, something like this:

parse("1 + 2 * 3")
==> [{type: 'num', val 1}, {type: 'op', val: '+'} ... and so on]

(I’ll leave implementing parse() as an exercise.)

That array you get is called a “token stream”. Now what you’re supposed to do next is take the token stream and turn it into an Abstract Syntax Tree, but I’m not teaching compiler construction today, so we can cheat and just turn those tokens back into the input string. They’ve all been validated to be “safe” for an eval, so now you can just stick them back into an eval

const source = tokens.map[tok => tok.val].join(' ') // assuming the same format as above
const result = eval(source)

It’s still dirty, but you’ve at least done a bit of real parsing, and you can construct a real expression parser from it later if you like. I’d encourage you to do so, it’s an endless rabbit hole of fun. You could end up writing a programming language :wink:

As an afterword, remember when Java Applets were a big-time security hole because they could run arbitrary code? That was an example of “validate then run”. And Java’s validator was absolutely state-of-the-art. Now they actually were doing proper parsing in a sense, but their problem was that the input language (java bytecode) was too complex to cover every case (mathematically, a full-blown programming language is infinitely complex), so they let through things that were clever enough to fool the bytecode validator. And now Java applets are dead (good riddance).

I agree with you; if you have come this far in FCC, chances are you are absolutely able to solve such a problem since you have solved similar ones in previous FCC challenges.
The main focus of this project is to expose you to other areas of javascript and not specifically the algorithms scripting which was covered.
Simply, there is a lot to learn. A lot. lol