How to create a mathematical expression from operators and operands in a queue

Question

I'm trying to build a mathematical expression (including parenthesis) with the operators and operands included in a queue.

Here is my code:

string createExp (queue<char> q) {
    string s;
    string s1, s2;
    char c;

    while (!q.empty()) {

        c = q.front();

        if (c == 'x') {
            s += "x";
            q.pop();
        }

        else if (c == 'y') {
            s += "y";
            q.pop();
        }

        else if (c == 'a') {
            s += "avg(";
            q.pop();

            s1 = createExp(q);
            q.pop();
            s2 = createExp(q);
            q.pop();
            s += s1;
            s += ',';
            s += s2;
            s += ')';
        }

        else if (c == 's') {
            s += "sin(pi*";
            q.pop();
            op++;
        }
        else if (c == 'c') {
            s += "cos(pi*";
            q.pop();
            op++;
        }
        else {
            s += "*";
            q.pop();
        }
    }

    while (op > cp) {
        s += ")";
        cp++;
    }

    return (s);
}

As you can see, in the case of the average (avg), I'm trying to call recursively to the function to obtain the next sequence of values.

For example, if my queue contains the next values:

s m a y x y

The expression should be like this:

sin(pi*(avg(y,x)*y)

But my code return this sequence:

sin(pi**avg(yyx)yyxyyx

Could you help me with this?

Thank you very much.

Have you tried to debug step by step? – sop Dec 16 '15 at 16:14 — sop, Dec 16 '15 at 16:14
Or at least with shorter/simpler inputs? – Ben Voigt Dec 16 '15 at 16:15 — Ben Voigt, Dec 16 '15 at 16:15
Where are your `op` and `cp` defined? Are they global? – sop Dec 17 '15 at 10:37 — sop, Dec 17 '15 at 10:37

score 3 · Answer 1 · answered Dec 16 '15 at 16:12

3

This part of processing avg(-,-) is horribly broken:

s1 = createExp(q);
q.pop();
s2 = createExp(q);
q.pop();

You're passing the queue by value, which creates a copy of it. Then you can't find out how many times the recursion popped the queue. But you magically assume that you should remove exactly one element. What if one of the arguments had a function call or operator in it.

To make matters worse, the recursion processes the entire rest of the queue, not just one expression.

answered Dec 16 '15 at 16:12

Ben Voigt

277,958
43
419
720

So, in your opinion, which would be the best way to solve this problems? Maybe use recursion in all the function? – elena.bdc Dec 16 '15 at 16:21
Yes, recursion, no loop, and I would pass the queue by reference so that the recursive calls control how many operands get removed. – Ben Voigt Dec 16 '15 at 16:33
And how could be this function? – elena.bdc Dec 16 '15 at 17:19

gt6989b · Answer 2 · 2015-12-16T16:13:17.580

1

The ** comes from both m in the string and you explicitly writing sin(pi* in the code.

In addition, the recursion into avg (when creating s1 seems to index the entire expression so you are getting yyx). You must make sure it only reads 1 complete expression out of the stack, not the rest of the thing. This is tricky because you need to tell the difference between avg(x,y) and avg(x+y,y*x) for example.

edited Dec 16 '15 at 16:13

answered Dec 16 '15 at 16:07

gt6989b

4,125
8
46
64

It comes from `m` putting the `*` into the output string immediately, instead of parsing two operands and putting `*` between them. – Ben Voigt Dec 16 '15 at 16:14
@BenVoigt yes, didn't i write the same thing? – gt6989b Dec 16 '15 at 16:17

score 0 · Answer 3 · answered Dec 17 '15 at 10:55

I have done a small modification of your code and it works perfectly:

int cp = 0,  op = 0;

std::string createExp(std::queue< char >& q)
{
    std::string s;
    std::string s1, s2;
    char c;

    while (!q.empty())
    {
        c = q.front();

        if (c == 'x')
        {
            s += "x";
            q.pop();
        }
        else if (c == 'y')
        {
            s += "y";
            q.pop();
        }
        else if (c == 'a')
        {
            s += "avg(";
            q.pop();

            s1 = q.front(); // here
            q.pop();
            s2 = q.front(); // and here
            q.pop();
            s += s1;
            s += ',';
            s += s2;
            s += ')*';
        }
        else if (c == 's')
        {
            s += "sin(pi*";
            q.pop();
            op++;
        }
        else if (c == 'c')
        {
            s += "cos(pi*";
            q.pop();
            op++;
        }
        else
        {
//             s += "*";
            q.pop();
        }
    }

    while (op > cp)
    {
        s += ")";
        cp++;
    }

    return (s);
}

But this will work only if your operator is always *. If you need also the other operators, then you need a more complex thing.

score 0 · Accepted Answer · answered Dec 17 '15 at 12:10

Here is my final recursive solution:

int cp = 0,  op = 0;

string recursiveExp (queue<char>& q) {
    char e;

    if (!q.empty()) {
        e = q.front();
        if (e == 'x' || e == 'y') {
            q.pop();
            s += e;
        }

        else if (e == 's') {
            q.pop();
            s += "sin(pi*";
            op++;
            recursiveExp(q);
            s += ")";
            cp++;
        }

       else if (e == 'c') {
            q.pop();
            s += "cos(pi*";
            op++;
            recursiveExp(q);
            s += ")";
            cp++;
        }

        else if (e == 'a') {
            q.pop();
            s += "avg(";
            op++;
            recursiveExp(q);
            s += ",";
            recursiveExp(q);
            s += ")";
            cp++;
        }

        else if (e == 'm'){
            q.pop();
            s += "(";
            op++;
            recursiveExp(q);
            s += "*";
            recursiveExp(q);
            s += ")";
            cp++;
        }
    }
    return s;
}

Thanks to everyone :)

How to create a mathematical expression from operators and operands in a queue

4 Answers4