0

I am creating a calculator in which I get a string from an input field. That string could for example be 2+3*9/3-6.

Now I want to perform multiplication and division operations first, so I will get 2+ 9 -6 where 9 will be the answer of the higher precedence operations.

So far I have done this. It is a complete function:

function calculateResult(){
    var higher = /(\d+[*\/]\d+)/g;
    calstr = $('#input-display').val();
    var m = '*';
    var res = calstr.split(higher);
    console.log(res.length+ ' : ' + res);
    // console.log(res);
    if(res.length > 2){
        console.log('split success');
        var index = 0;
        do{
            for(var i = 1; i < res.length; i+=2){
                var a = res[i].split(/([0-9\.]+)/g);
                console.log('before calculation : '+calstr);
                var regex = RegExp('(?:.*?(\\d+[*\/]\\d+)){' + i + '}');
                console.log('regex generated : '+regex);
                if(a[2] == '*'){
                    calstr = calstr.replace(regex , Number(a[1])*Number(a[3]));
                }
                else if(a[2] == '/'){
                    calstr = calstr.replace(regex , Number(a[1])/Number(a[3]));
                }
                console.log('replaced result : ' + calstr);
                console.log('replaced result at : ' + calstr.search(regex));
            }
            // console.log('result : ' + calstr);
            console.log('multiplication complete');
            res = calstr.split(higher);
            console.log('result is : '+res);
        }while(res.length > 2);
    }
    else if(res.length == 1)
        console.log('split failed');
}

When I try to replace the result inside a string, the replace succeeds. But on some several inputs it replaces the entire string that was passed as argument to this function, which is not what I want.

What did I do wrong?

If still something is unclear, I am happy to give clarifications.

After multiplication and division runs smoothly, I plan to change these regular expressions, to add support for addition and subtraction. It would use the same code but with a next run for the lower precedence operators.

trincot
  • 317,000
  • 35
  • 244
  • 286
Syed Naeem
  • 681
  • 8
  • 17
  • It would've been better if you would pin-point a shorter code snipset that actually fails. On a side note, are you required to do expression parsing yourself or simply calling eval( _your_string_ ) would suffice?[link](http://www.w3schools.com/jsref/jsref_eval.asp) – Vladimir M Sep 19 '16 at 07:07
  • This has little to do with jQuery. You only use it to fetch the input string (`$('#input-display').val();`), which is not really relevant to the question. – trincot Sep 19 '16 at 07:21
  • See also http://stackoverflow.com/questions/32982719/chrome-app-doing-maths-from-a-string/ – guest271314 Sep 19 '16 at 07:28
  • See https://jsfiddle.net/ycaybsLx/ for a simpler approach. Your code logic is not appropriate: you are trying to match a specific occurrence of a number with `RegExp('(?:.*?(\\d+[*\/]\\d+)){' + i + '}')` but once you replace the first pair, the second one will become first, and so on. Besides, you are replacing the whole match, while you intend to only replace the `number`+`[/*]`+`number`. – Wiktor Stribiżew Sep 19 '16 at 07:43
  • i have come to understand the this regex is replacing the from 0 to the occurance with the answer... but what i am trying to do is only replace multiplication with the answer... of i remove those ? mark part... i cant find the nth occurance which i want to replace.. – Syed Naeem Sep 19 '16 at 07:49

3 Answers3

2

Wouldn't it be simpler to just eval the input?

function doEval() {
  try {
    alert(eval(document.querySelector('input').value));
  } catch (error) {
    alert('Invalid input: ' + error);
  }
}
<input type='text' value='1+(4-3)*10/5' />
<input type='button' onclick='doEval();' value='Eval' />
Jonathan
  • 8,771
  • 4
  • 41
  • 78
  • this never crossed my mind... but it does solve the problem... i was sofocused on regex...but i has helped me improve my regex skills... thanks again... . it always nice to have more than 1 solutions – Syed Naeem Sep 19 '16 at 08:53
1

This regex is the cause of the wrong result you get:

RegExp('(?:.*?(\\d+[*\/]\\d+)){' + i + '}');

It should not match the .*?, as that will make you remove all preceding characters in the replace that follows.

Also, the single \ is unnecessary. If you really wanted to escape the / for the regular expression, you would have to double the preceding backslash, because it also needs to be escaped in the quoted string. But since the forward slash does not occur in a regular expression literal (like /.../g), it does not have to be escaped at all.

The {' + i + '} part is not useful. This only requires that a pattern occurs that many times, but that could be false after you have made already several replacements. In fact, you need to match the first occurrence every time, since all previous ones were already replaced.

So it should be just this:

    RegExp('(\\d+[*/]\\d+)');

... and so it is in fact no different from the higher regex, except for the global modifier. But that global modifier on higher is not necessary, as split searches all occurrences anyway, even without that modifier. This means you actually only need higher (without g), and not the regex above.

Some other things to improve:

  1. The following debugging line is not of great use, because calstr was already modified, while the regex was applied before the modification:

    console.log('replaced result at : ' + calstr.search(regex));
    
  2. It is better not to access the input element input-display inside the function. Instead pass calstr as a function argument. This makes it independent of your browser context.

  3. Number(...) can be done shorter with the unitary plus (+).

Here is the updated code, which also implements the + and - processing, by putting the whole thing in another loop which has 3 iterations: one for * and /, and two others for - (which must come before +!) and +. This had another complexity: a unitary minus should not be confused with the normal minus operator. You'll see that managed with (?:^-)? in the inner regular expression.

function calculateResult(calstr){
    for (var level = 0; level < 3; level++) {
        var search = level == 0 ? '(\\d+[*/]\\d+)'
                   : level == 1 ? '(-?\\d+-\\d+)'
                                : '(-?\\d+\\+\\d+)';
        var higher = RegExp(search);
        var res = calstr.split(higher);
        while(res.length > 2){
            for(var i = 1; i < res.length; i+=2){
                var a = res[i].split(/((?:^-)?[0-9\.]+)/g);
                calstr = calstr.replace(higher, 
                     a[2] == '*' ? +a[1] * +a[3]
                   : a[2] == '/' ? +a[1] / +a[3]
                   : a[2] == '+' ? +a[1] + +a[3]
                                 : +a[1] - +a[3]
                );
            }
            res = calstr.split(higher);
        }
    }
    return res[0];
}

$('button').click(function () {
   var input = $('#input-display').val();
   var result = calculateResult(input);
   $('span').text(result);
});
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<input id="input-display" value="2+3*9/3-6">
<button>Calc</button><br>
Result: <span></span>

Note that you are re-inventing the wheel, as JavaScript offers the eval function to evaluate (controlled) expressions like that. Moreover, this code will not evaluate more complex expressions, like when they have parentheses.

trincot
  • 317,000
  • 35
  • 244
  • 286
  • thankyou very much.... only doing the regex fixing my problem seems to be solved... that is the solution i was looking for... but i will try to understand the improvement you wrote, and perfact my code. thanks again – Syed Naeem Sep 19 '16 at 08:48
  • You're welcome. Please take note of a modification I made just now. It concerns the `{ + i + }` part: it can be excluded. – trincot Sep 19 '16 at 08:54
0

Seems like your first regex var higher = /(\d+[*\/]\d+)/g; is not retrieving "3*9/3" from 2+3*9/3-6 (checked in https://regex101.com/#javascript)

Try this changing it to

var higher = /(\d[^\+\-]*)+/g;
weizong song
  • 515
  • 4
  • 9
  • in first iteration it matches 3*9 and calculates result... which is 27 and then replaces it which will give " 2+27/3-6 "... next iteration it will find 27/3 and replace it with 9.... then we will have " 2+9-6 "... global is working as i intented... but problems lies in when i try to replace the 27 with 3*9 and then replace 27/3 with 9... it replaces 0-to-occurance . all of it... i cant seem to solve that. most inner loop have a regex that should replace the nth occurance.. but it is not working as i intented... so if i am not mistaken.problem is with the inner regex – Syed Naeem Sep 19 '16 at 08:20