6

In the code I use an expression tree "3 + 2.53 - 1.75" that should return a result of 3.78. However, it ends up adding all the values within the string and outputs 7.28. I ran through the code multiple times on paper trying to see what happens in each iteration of the for loop where the index variables i and distance_operator are used too. As far as I have gone through it I cannot find a reason why the program continues to add each float value. By the time the '-' character is reached it should subtract the next value.

The distance_operator is used to be an offset from the first operator where index i will be pivoted so that I can take a portion of that string and calculate it using the substr() function. 

    float total = (float)value(expression[0]);
    int distance_operator;

    for (i = 1; i < expression.size(); i++) {
        if (expression[i] == '+' || expression[i] == '-') {
            distance_operator = i + 1;
            while (expression[distance_operator] != '+' || expression[distance_operator] != '-') {
                distance_operator++;
                if (distance_operator == expression.size())
                    break;
            }
            if (expression[i] == '+')
                total += std::stof(expression.substr(i, distance_operator - i));
            else if(expression[i] == '-')
                total -= std::stof(expression.substr(i, distance_operator - i));

        }
    }
Code4life
  • 95
  • 4
  • Have you tried to [debug your program](https://ericlippert.com/2014/03/05/how-to-debug-small-programs/)? Do the `substr` function return the correct sub-string? For some input, what is the expected output? And what is the actual output? – Some programmer dude May 17 '19 at 05:58
  • 1
    Off-topic, but this is not a good approach to parsing expressions like this. Hopefully you won't get asked to now add parentheses to the expressions and/or multiplication and division -- you will quickly see that this code would now be practically worthless. – PaulMcKenzie May 17 '19 at 06:03
  • @PaulMcKenzie: agree, but indeed you can get quite far anyway. For example I remember my first expression computing algorithm (written in basic when I was 15) and I was looking for the first closed parenthesis, then going back to the first open parenthesis and calling a sub for the sub-expression and replacing it in the original string. The sub was looking for `+` and `-` and calling a sub for whatever was inbetween and this second sub was handling `*` and `/`. Not the best way for sure... but you can get there... :-D – 6502 May 17 '19 at 06:07
  • @6502 -- Yes, but I'm letting the OP know there is a very good chance he needs to start from scratch again or go completely crazy trying to adjust the current program, even for the simplest of features added to the expression. There are formal ways of doing this, where having to start all over not be the case. This is the kind of assignment that sets up a trap for the unaware student. – PaulMcKenzie May 17 '19 at 06:11
  • @PaulMcKenzie: I'm not so sure that teaching this guy/girl **now** about say ANTLR is going to make him/her a better programmer. – 6502 May 17 '19 at 06:19
  • Well, if the OP comes back a week later and has close to a mental breakdown when asked to add some feature, they will remember this thread. Just saying... – PaulMcKenzie May 17 '19 at 06:21
  • I truly appreciate all the advice in my initial post, but how would I go about optimizing my program where it ends up only consisting a few lines of code and completes the objective since it seems there exists a solution like that for these tree expression coding assignments? I do agree with @PaulMcKenzie that there probably is a better way and that my code is redundant, but I am unaware of a better solution for these problems. – Code4life May 17 '19 at 06:38
  • There is a parentheses sub-problem and the string also includes multiplication and division. I have gotten the multiplication/division arithmetic correct with the function above, I did not include the part for solving the string with ```*``` and ```/``` but I ran the code and it works with those operators. However, I have not gotten to the parentheses challenge yet, but couldn't I use the same logic used in the function above? Look for the ```(``` character in the string and then have the second index variable to iterate through the string until a ```)``` is encountered. – Code4life May 17 '19 at 06:41
  • If you are curious to how I initially solved this was to make two for loops where the first looks for ```*``` and ```/``` characters and removes the operands and the operator from the string then replaces it with the calculated value so that order of operations will be articulated through arithmetic calculation. That can also be used with parentheses if I am not mistaken although I did not get to that part yet. I can simply extract the portion of the string enclosed by parentheses and insert the calculated value until the remaining string is only addition and subtraction operators. – Code4life May 17 '19 at 06:44
  • @Code4life: starting with `(` and looking for `)` is not going to work (parenthesis can be nested... e.g. `1 + 2*(3 + 4*(5+6))`. Something that can work is looking for `)` first and then going back to closest `(`. – 6502 May 17 '19 at 07:40
  • In the assignment we don't have to test for closed parentheses, therefore would it change anything if ```(``` is searched first? – Code4life May 17 '19 at 08:44
  • @Code4life: First `(` it doesn't match with first `)` but with last. The key issue however is that the found substring can contain other parenthesized subexpressions so you need to implement a recursive call. If you look for first closed parenthesis and last open parenthesis instead the subpart is parenthesis-free and you can process just operations inside it. – 6502 May 17 '19 at 09:22
  • @Code4life -- The better solution is to change the infix expression to postfix, and then do the calculation via stack. What you're trying to do is to do things symbolically, i.e. like how it is done in elementary school. Writing programs that mimic how humans would solve the expression is much more difficult than, say, doing an infix to postfix conversion and computing the result. The order of operations and parenthese handling just comes naturally with postfix/stack implementation, so that wouldn't be an issue. – PaulMcKenzie May 17 '19 at 12:57

2 Answers2

5

The code is almost correct but there is an "off-by-one" error.

The problem is that when finding the - the right substring used will be "- 1.75" with a negative value when parsed as a number and you will be subtracting it, basically negating the value you wanted to use. The accumulating code should be:

if (expression[i] == '+')
    total += std::stof(expression.substr(i+1, distance_operator-i-1));
else if(expression[i] == '-')
    total -= std::stof(expression.substr(i+1, distance_operator-i-1));

Note that i+1 is used, so the expression sign found will be skipped.

6502
  • 112,025
  • 15
  • 165
  • 265
1

Also note that this check

while (expression[distance_operator] != '+' || expression[distance_operator] != '-')

will always be true, because a thing is always different from A OR different from B. The correct logical operator is &&.

w513894
  • 11
  • 1