while loop over and over in javascript

Question

I am writing code of calculator considering the priority of operation. What I am trying to do is to replace the priority that matches the regular expression I wrote and calculate it, and loop through it until the length of string, which is the input initially, becomes 1.

The below is my code, but running it results in an infinite loop.

const Calculator = function() {
    this.evaluate = string => {
    let regex = /\((\d+)\s[*/+-]\s(\d+)\)|(\d+)\s[*/]\s(\d+)|(\d+)\s[+-]\s(\d+)/g

        while(string.length > 1) {
            string.replace(regex, cal);
        }
        return string;
    }
};

function cal(argument) {
    let temp = argument.split(' ');
    let regexP = /\((\d+)|(\d+)\)/g

    if(temp[0].match(regexP)) {
        temp[0] = temp[0].slice(1);
        temp[2] = temp[2].slice(0, 1);
    }
    switch(temp[1]) {
        case '+': return +temp[0] + +temp[2];
        case '-': return +temp[0] - +temp[2];
        case '*': return +temp[0] * +temp[2];
        case '/': return +temp[0] / +temp[2];
        default: return undefined;
    }
}

let calculate = new Calculator()
console.log(calculate.evaluate("2 / 2 + 3 * 4 - 6"));

For some reason, the code is looping over and over and isn't returning a value.

What am I doing wrong, and how can I fix it?

Answer 1

You need to

(1) assign the result of calling .replace to the string (Javascript strings are immutable):

string.replace(regex, cal);

(2) The resulting string could have all +-*/ operations finished but still have a length larger than 1, eg, if the result was 3 * 4 resulting in 12. Use while(/[-+*/]/.test(string)) { instead:

const Calculator = function() {
    this.evaluate = string => {
    let regex = /\((\d+)\s[*/+-]\s(\d+)\)|(\d+)\s[*/]\s(\d+)|(\d+)\s[+-]\s(\d+)/g

        while(/[-+*/]/.test(string)) {
            string = string.replace(regex, cal);
        }
        return string;
    }
};

function cal(argument) {
    let temp = argument.split(' ');
    let regexP = /\((\d+)|(\d+)\)/g

    if(temp[0].match(regexP)) {
        temp[0] = temp[0].slice(1);
        temp[2] = temp[2].slice(0, 1);
    }
    switch(temp[1]) {
        case '+': return +temp[0] + +temp[2];
        case '-': return +temp[0] - +temp[2];
        case '*': return +temp[0] * +temp[2];
        case '/': return +temp[0] / +temp[2];
        default: return undefined;
    }
}

let calculate = new Calculator()
console.log(calculate.evaluate("2 / 2 + 3 * 4 - 6"));

You can also improve the code in cal by matching and destructuring the left digits, operator, and right digits in one go, with

`const [, left, operator, right] = matchedSubstr.match(/(\d+) ([-+*/]) (\d+)/);

const Calculator = function() {
    this.evaluate = string => {
    let regex = /\((\d+)\s[*/+-]\s(\d+)\)|(\d+)\s[*/]\s(\d+)|(\d+)\s[+-]\s(\d+)/g

        while(/[-+*/]/.test(string)) {
            string = string.replace(regex, cal);
        }
        return string;
    }
};

function cal(matchedSubstr) {
    const [, left, operator, right] = matchedSubstr.match(/(\d+) ([-+*/]) (\d+)/);
    switch(operator) {
        case '+': return +left + +right;
        case '-': return +left - right;
        case '*': return +left * right;
        case '/': return +left / right;
    }
}

let calculate = new Calculator()
console.log(calculate.evaluate("2 / 2 + 3 * 4 - 6"));

Answer 2

CertainPerformance has already pointed out the mistakes that led to the undesired behaviour.

In order to get the priorities right, you should first resolve all parentheses, then the multiplications/divisions and then the additions/subtractions.

You could use three regular expressions for this (one for each), and use recursion to deal with any expression within parentheses. Note that such a sub-expression does not need to be just one operation. It could well have more operations and even parentheses. So parentheses can best be resolved from the inside out.

Here is how you could adapt your code for doing that:

const Calculator = function() {
    const cal = (a, op, b) => {
        switch(op) {
            case '+': return +a + +b;
            case '-': return a - b;
            case '*': return a * b;
            case '/': return a / b;
            case ')': return +this.evaluate(a); // Use recursion to resolve parentheses
        }
    };

    this.evaluate = string => {
        let regexes = [
            /\(([^()]+)(\))/g, // Priority 1: parentheses
            /(-?\d+(?:\.\d+)?)\s*([*\/])\s*(-?\d+(?:\.\d+)?)/, // Priority 2: multiplication & division
            /(-?\d+(?:\.\d+)?)\s*([+-])\s*(-?\d+(?:\.\d+)?)/, // Priority 3: addition & subtraction
        ];
        for (let regex of regexes) {
            let found;
            do {
                found = false;
                string = string.replace(regex, (_, ...args) => {
                    found = true;
                    return cal(...args).toFixed(12);
                });
            } while (found);
        }
        return string;
    };
};

let calculate = new Calculator()
console.log(+calculate.evaluate("2 / 2 + 3 * 4 - 6"));

The regular expressions allow for decimal points (as division may lead to fractional numbers), and negative numbers (as subtraction may yield that). So (\d+) became (-?\d+(?:\.\d+)?)

To avoid scientific notation getting inserted into the string, I use toFixed(12), assuming that this will be enough as precision.

For more efficiency look into the Shunting-yard algorithm. For supporting more operators, functions and precendence-settings in your string parsing, you could take inspiration from this answer