2
\$\begingroup\$

I've written code that will take any basic math sum input as a string (for instance, 2*3+4/4) and solve it. The way I achieve this is by splitting the string into sub-strings, with multiplication and division being done first (based on the regular order of operations). So in our example sum we would be left with 2*3 and 4/4.

It then solves these sums and replaces the original sub-string with the answer : 2*3+4/4 would become 6+1. The numbers left in the addition string are then added into an array. All elements in the array are then added.

The part of my code in question is the function that identifies if there are any multiplication or division operations left to do, and if so, performs them. I feel like this function is very long and cumbersome and needs to be shortened, hence why I'm posting it here on CodeReview.

void mathInterpreter::format()
{
    BasicQuestion TempQuestion;
    basic_ops *solver = new basic_ops();

    //If we have multiplication or division in our sum then we should do this loop.
    for (int i = 0; MultDevCount != 0; i++)
    {
        //get some information about the question we are on, and create a temp string.
        op_positions temp;
        temp = positions_d[i];
        std::string temp_str;

        double MyAnswer;

        //if the operation that we are on in the for loop happens to be a multiply or divide, then lets do that.
        if (temp.operation == MULTIPLY || temp.operation == DIVIDE)
        {
            if (operation_count == 1) /* ONLY ONE OPERATION */
            {
                temp_str = user_string;
            }
            else if (positions_d[i].operation_at == positions_d.front().operation_at) /* FIRST OPERATION IN SUM */
            {
                temp_str = get_sum(temp, 0, positions_d[i + 1].operation_at);
            }
            else if (positions_d[i].operation_at == positions_d.back().operation_at) /* LAST OPERATION IN SUM */
            {
                temp_str = get_sum(temp, positions_d[i - 1].operation_at, 0);
            }
            else /* THE OPERATION IS IN THE MIDDLE OF THE SUM */
            {
                temp_str = get_sum(positions_d[i], positions_d[i - 1].operation_at, positions_d[i + 1].operation_at);
                if (positions_d[i - 1].operation == SUBTRACT) temp_str = SUBTRACT_SYM + temp_str;
            }

            //setup the return question properly and then send it to the solver.
            TempQuestion = quest_from_tuple(get_nums(temp_str));

            answer SumAns;


            SumAns = solver->performCalc(TempQuestion);
            MyAnswer = SumAns.answer;

            if (SumAns.error)
            {
                error = true;
                delete TempQuestion.numbers;
                return;
            }

            //replace the original sum string with it's answer string.
            user_string = replace_strings(std::to_string(MyAnswer), temp_str, user_string);

            delete (TempQuestion.numbers);

            //sort operation counts.
            operation_count = CountOperations();
            positions_d.clear();
            populate_op_positions();

            //an operation has been removed, so positions_d[i] may have changed. If positions_d[i] is still a mult or dev, we need to
            //redo this loop with the same value for i. So subtract one so that the loop is redone with the same value.
            if (positions_d[i].operation == MULTIPLY || positions_d[i].operation == DIVIDE) i--;

        }
    }


}

Some additional information: positions_d is a deque which holds struct elements - the struct contains the position of an operation and what it is.

The get_sum() function takes in a struct element (same struct as mentioned above) and the positions of the operations before and after it. It will then return a string with two operands and the operation (e.g. 2*3).

\$\endgroup\$
3
  • 1
    \$\begingroup\$ Welcome to Code Review! I hope you get some great answers. \$\endgroup\$ – Phrancis Aug 7 '16 at 1:40
  • \$\begingroup\$ I'm curious, what you're describing here is similar to a parsing tree. You might want to build a tree where the vertices are operators and the leaves are operands. After you build the tree, traverse the tree replacing operators with the results of the operation. \$\endgroup\$ – pacmaninbw Aug 7 '16 at 1:44
  • \$\begingroup\$ Doing string manipulation for a maths parser is the wrong way to do this. You can probably write a bison/flex application to do this in about 10 lines. \$\endgroup\$ – Martin York Aug 7 '16 at 17:12
1
\$\begingroup\$

My advice here is not to use C++ (directly) for this type of job. There are tools out there to help you parse formatted streams (you have to learn another tool though).

Personally I use flex and bison. BUT I recommend you look around and see if there are more modern version or alternatives.

Flex Rules

This converts a stream of characters into a stream of tokens.

%%
\+              return '+';
\-              return '-';
\*              return '*';
\/              return '/';
\(              return '(';
\)              return ')';
{Number}        return 259;
{WhiteSpace}    /* Ignore */
.               throw std::runtime_error("Invalid Character");
%%

Bison Rules

This converts a stream of tokens and build a parse stack that is processed.

%%
result  :   exp                     {value = $1;}
exp     :   add_exp
add_exp :   mul_exp
        |   add_exp '+' mul_exp     {$$ = $1 + $3;}
        |   add_exp '-' mul_exp     {$$ = $1 - $3;}
mul_exp :  pri_exp
        |   mul_exp '*' pri_exp     {$$ = $1 * $3;}
        |   mul_exp '/' pri_exp     {$$ = $1 / $3;}
pri_exp :   '(' exp ')'             {$$ = $2;}
        |   Number
Number  :   NUMBER                  {$$ = getCurrentNumber();}
%%

The actual files are slightly bigger as there are some boiler plate that you need get everything working together. But this boilerplate is very similar so once you have it no need to worry about it. The languages are very concise and understandable (but that is relative).

A lot of research has already been done on parsing so there is very little need to reinvent the wheel.

\$\endgroup\$
1
  • \$\begingroup\$ Thank you for this response! Having a quick look into flex and bison, one of the examples I found on github is actually a math interpreter similar to what i've built here. Will definitely have a play around and work it into my code. \$\endgroup\$ – Swemoph Aug 15 '16 at 4:20

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.