Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

This was my first attempt at making a C++ calculator. The user basically inputs an expression (I'll use \$1+1*(9*(11-1))\$ as an example) and the program searches for and creates a vector for parentheses (which is a class I made which records its position in the string).

String:

[1][+][1][*][(][9][*][(][11][-][1][)][)]
 0  1  2  3  4  5  6  7  8   9 10 11 12

Parentheses Vector:

[( position = 4][( position = 7][) position = 11][) position = 12]
       0                 1               2               3

If in the parentheses vector, the parentheses' position = n, then the corresponding closing parentheses will be vector.size()-n.

The program deals with the first parenthesis and finds its corresponding closing parenthesis using the above statement and what I call the sub expression is found between the position member variable of the selected parentheses. For the innermost set of parentheses, the sub expression will be thus: 11-1

The expression will be evaluated (10) and the result will be placed in the expression:

\$1+1*(9*10)\$

and the same for the other sub expression:

\$1+1*90\$

In terms of operator precedence, the B in BOMDAS has been followed so far, so the other operators are to be used (*,/,+,- are all supported at the moment).

Each operator has an assigned value in a map called operatorClassMap:

operatorClassMap['*']=2;
operatorClassMap['/']=2;
operatorClassMap['+']=1;
operatorClassMap['-']=1;

A little explanation on operators at this point:

  1. The class operatorClass has two pointers to doubles in the expression.
  2. The class records the operator's "value" assigned above.
  3. I overloaded the < operator so that the sort() function will sort the operator vector for me.
  4. The vector is transferred to a stack and evaluated from the top.

To continue with the above example: \$1+1*90\$

The operator stack will look something like this:

[ 1 * 90]
[ 1 + 1]

The multiplication will be performed first resulting in 90. The program then decides whether or not to place the result of the multiplication to the left or right of the operator beneath it:

[1+90]

Then the final operation is performed, resulting in 91.

#include <iostream>
#include <sstream>
#include <string>
#include <stack>
#include <queue>
#include <vector>
#include <stdlib.h>
#include <map>
#include <algorithm>
#include <math.h>
#include <iomanip>
#include <limits>
using namespace std;

class operatorClass
{
public:
    operatorClass(char&,int&);
    void addOperands(long double&, long double&);
    bool operator < (const operatorClass& str) const;
    int value;
    char actualoperatorClass;

    long double* pointer_To_leftOperand;
    long double* pointer_To_rightOperand;

};
operatorClass::operatorClass(char &receivedoperatorClass, int &receivedValue)
{
    actualoperatorClass = receivedoperatorClass;
    value =  receivedValue;
}
void operatorClass::addOperands(long double &receivedleft, long double &receivedright)
{
pointer_To_leftOperand=&receivedleft;
//std::cout << " LEFT " << *pointer_To_leftOperand << std::endl;
    pointer_To_rightOperand=&receivedright;
//std::cout << " RIGHT " << *pointer_To_rightOperand << std::endl;
}
bool operatorClass::operator < (const operatorClass& str) const
{
    return (value < str.value);
}
inline void printOperatorVector(std::vector<operatorClass> VectorToPrint)
{
    for(unsigned int index = 0; index < VectorToPrint.size(); index++)
    {
     //std::cout << "Element #" << index <<": " << VectorToPrint[index].actualoperatorClass << std::endl;
    }
}
class Parenthesis
{
    public:
    Parenthesis(unsigned int,char);
    char typeOfParenthesis;
    unsigned int position;
    private:

};
Parenthesis::Parenthesis(unsigned int receivedPosition, char parenthesisType)
{
    position = receivedPosition;
    typeOfParenthesis = parenthesisType;
}
class Expression
{
    public:
    Expression(std::string,bool);
    long double result;
    private:



    bool hasBrackets;
    std::vector <Expression> subExpressions;
    std::vector<Parenthesis> vector_parentheses; //If the position of a parenthesis in a vector
    //is 'n', then the position of the closing parenthesis should be vectorSize-n in the
    //same vector
    void containsBrackets();//Checks to see if the expression contains parentheses
    void getParentheses(); //Gets position and types of parentheses in the expression
    void getSubExpressions(); //Gets the contents between each parenthesis so that they may be evaluated

    long double evaluate();
    std::string expressionString;
};
Expression::Expression(std::string expression, bool sub) //NOTE: being a sub means that it was surrounded
//by brackets '(' ')'
{


    hasBrackets = false;
    expressionString = expression;
    containsBrackets();
    // //std::cout << "Successfully complete 'containsBrackets' function" << std::endl;
    if (hasBrackets == true)
    {
     getParentheses();
     // //std::cout << "Successfully complete 'getParentheses()' function" << std::endl;
    getSubExpressions();
    // //std::cout << "Successfully complete 'getSubExpressions()' function" << std::endl;
    }
    evaluate();
    // //std::cout << "Successfully complete 'evaluate()' function" << std::endl;


}
long double Expression::evaluate()
{
std::map<char, unsigned int> operatorClassMap;
operatorClassMap['*']=2;
operatorClassMap['/']=2;
operatorClassMap['+']=1;
operatorClassMap['-']=1;
std::vector<long double> numbers;
std::vector<operatorClass> operatorClasss;
long double number = 0;
std::string numberString = ""; //For having multi-digit numbers
// //std::cout << "Working expression: " << expressionString << std::endl;

for(unsigned int index = 0; index<=expressionString.size(); index++)
{
    if(expressionString[index] != '+' && expressionString[index] != '-' && expressionString[index] != '*' && expressionString[index] != '/' && expressionString[index] != ' ')
    {
        numberString+= expressionString[index];
    }
    if (expressionString.size() == index)
    {
    number= strtod(numberString.c_str(), NULL);
    numbers.push_back(number);
    numberString = "";

    }
    if(expressionString[index] == '+' || expressionString[index] == '-' || expressionString[index] == '*' || expressionString[index] == '/' || expressionString[index] == ' ')
    {
    number= strtod(numberString.c_str(), NULL);
    numbers.push_back(number);
    numberString = "";

    }
}
//std::cout << "SIZE" << numbers.size() << std::endl;
for(unsigned int index = 0; index < numbers.size(); index++)
{
//std::cout  << "NUMBER" << numbers[index] << std::endl;
}

for(unsigned int index = 0; index<expressionString.size(); index++)
{
//std::cout << "Index :" << index << std::endl;
    if(expressionString[index] == '+' || expressionString[index] == '-' || expressionString[index] == '*' || expressionString[index] == '/' || expressionString[index] == ' ')
    {
     int value = operatorClassMap[expressionString[index]];
     if(numbers.size() > 2)
     {
         operatorClass tempoperatorClass(expressionString[index],value);
         operatorClasss.push_back(tempoperatorClass);
     }

     else
     {
        operatorClass tempoperatorClass(expressionString[index],value);
        operatorClasss.push_back(tempoperatorClass);
     }

    }
    else
    {

    }
}
for(unsigned int index = 0; index < operatorClasss.size(); index++)
{
if(numbers.size() >= 2)
operatorClasss[index].addOperands(numbers[index],numbers[index+1]);
else
operatorClasss[index].addOperands(numbers[0],numbers[1]);
}

std::sort(operatorClasss.begin(),operatorClasss.end());



for(unsigned int index = 0; index < numbers.size(); index++)
{
//std::cout << numbers[index] << std::endl;
}
printOperatorVector(operatorClasss);
//std::cout << 7 << std::endl;
std::stack<long double> numberStack;
std::stack<operatorClass> operatorClassStack;

for (unsigned int index = 0; index < operatorClasss.size(); index++)
{
operatorClassStack.push(operatorClasss[index]);
}
// //std::cout << "Successfully added operatorClasss and numbers to stacks" << std::endl;
long double Result = 0;
for(unsigned int index = operatorClassStack.size();index>0;index--)
{
    unsigned int previousValue = operatorClassMap[operatorClassStack.top().actualoperatorClass];
    if (operatorClassStack.top().actualoperatorClass == '*')
    {

         //std::cout << "Top number: " << *operatorClassStack.top().pointer_To_leftOperand << std::endl;

         //std::cout << "operatorClass: " << operatorClassStack.top().actualoperatorClass;

         //std::cout << "Other number: " << *operatorClassStack.top().pointer_To_rightOperand << std::endl;
//change stack to vector
        Result = *operatorClassStack.top().pointer_To_leftOperand* *operatorClassStack.top().pointer_To_rightOperand;
        numberStack.push(Result);
        if(operatorClassStack.empty() == false)
        operatorClassStack.pop();

          //std::cout << "RESULT! : "<< Result << std::endl;
    }
    else if (operatorClassStack.top().actualoperatorClass == '/')
    {

         //std::cout << "Top number: " << *operatorClassStack.top().pointer_To_leftOperand << std::endl;

        //std::cout << "operatorClass: " << operatorClassStack.top().actualoperatorClass;

         //std::cout << "Other number: " << *operatorClassStack.top().pointer_To_rightOperand << std::endl;

        // //std::cout << 1+1;
        Result = *operatorClassStack.top().pointer_To_leftOperand/ *operatorClassStack.top().pointer_To_rightOperand;
        numberStack.push(Result);
        // //std::cout << Result;
        if(operatorClassStack.empty() == false)
        operatorClassStack.pop();

          //std::cout << "RESULT! : "<< Result << std::endl;
    }

    else if (operatorClassStack.top().actualoperatorClass == '+')
    {

         //std::cout << "Top number: " << *operatorClassStack.top().pointer_To_leftOperand << std::endl;

         //std::cout << "operatorClass: " << operatorClassStack.top().actualoperatorClass;

         //std::cout << "Other number: " << *operatorClassStack.top().pointer_To_rightOperand << std::endl;

        Result = *operatorClassStack.top().pointer_To_leftOperand + *operatorClassStack.top().pointer_To_rightOperand;
        numberStack.push(Result);
        if(operatorClassStack.empty() == false)
        operatorClassStack.pop();

          //std::cout << "RESULT! : "<< Result << std::endl;


    }
    else if (operatorClassStack.top().actualoperatorClass == '-')
    {

         //std::cout << "Top number: " << *operatorClassStack.top().pointer_To_leftOperand << std::endl;

         //std::cout << "operatorClass: " << operatorClassStack.top().actualoperatorClass;

         //std::cout << "Other number: " << *operatorClassStack.top().pointer_To_rightOperand << std::endl;

        Result = *operatorClassStack.top().pointer_To_leftOperand- *operatorClassStack.top().pointer_To_rightOperand;
        numberStack.push(Result);
        if(operatorClassStack.empty() == false)
        operatorClassStack.pop();

          //std::cout << "RESULT! : "<< Result << std::endl;
    }
    if(operatorClassStack.size() > 0)
    {
        if(operatorClassMap[operatorClassStack.top().actualoperatorClass] > previousValue)
        operatorClassStack.top().pointer_To_leftOperand =  &Result;

        if(operatorClassMap[operatorClassStack.top().actualoperatorClass] <= previousValue)
        operatorClassStack.top().pointer_To_rightOperand =  &Result;
    }




}
// //std::cout << "Successfully evaluated expression" << std::endl;
result = numberStack.top();
// //std::cout << result;
// //std::cout << "Successfully evaluated expression" << std::endl;
}
void Expression::containsBrackets()
{
    for(unsigned int index = 0; index < expressionString.size(); index++)
    {
        if (expressionString[index]=='(' ||expressionString[index]==')' )
        {
            hasBrackets = true;
        }
    }
}
void Expression::getParentheses() //Finds the parentheses and their positions in the expression
//so that their contents can be converted to sub expressions.
{
    for (unsigned int index = 0; index < expressionString.size(); index++)
    {
        if (expressionString[index] == '(' || expressionString[index] == ')')
        {
            Parenthesis temporary(index, expressionString[index]); //Stores the position and type of the parenthesis
            vector_parentheses.push_back(temporary);
        }
    }

    /*for (unsigned int index = 0; index < vector_parentheses.size(); index++)
    {
        // //std::cout << vector_parentheses[index].typeOfParenthesis << std::endl;
        // //std::cout << vector_parentheses[index].position << std::endl;
    }
    */
}
void Expression::getSubExpressions()
{
    for(unsigned int index = 0; index < vector_parentheses.size(); index++)
    {
        if(vector_parentheses[index].typeOfParenthesis == '(')
        {
            std::string subExpression = "";
            // | ( | ( | ( | ) | ) | ) |
            //   *   -   -   -   -   *
            //  n=0                 =size-n
            //in an array of parentheses, corresponding closing parenthesis for an opening bracket at position in is
            // at [size-n]

            unsigned int positionOfOpeningParenthesis = vector_parentheses[index].position;
            // //std::cout << "Opening parenthesis: " << positionOfOpeningParenthesis << std::endl;
            // //std::cout << "successfully got opening parenthesis position" << std::endl;
            unsigned int positionOfClosingParenthesis = vector_parentheses[vector_parentheses.size()-1 - index].position;
            // //std::cout << "Closing parenthesis: " << positionOfClosingParenthesis << std::endl;
            // //std::cout << "successfully got closing parenthesis position" << std::endl;
            for(unsigned int index2 = positionOfOpeningParenthesis+1; index2 < positionOfClosingParenthesis;index2++)
            {
                subExpression+=expressionString[index2]; //gets stuff found between each bracket
            }
            // //std::cout << "successfully got expression" << std::endl;
            // //std::cout << "Sub expression: " << subExpression;
            Expression temporaryExpression(subExpression, true); //creates a new sub expression
            // //std::cout << "successfully created new expression" << std::endl;
            int digits_before = 1 + (int)floor(log10(fabs(temporaryExpression.result)));
            int digits_after = std::numeric_limits<long double>::digits10 - digits_before;

            long double whole = floor(pow(10, digits_after) * fabs(temporaryExpression.result) + 0.5);
            while (digits_after > 0 && (whole/10.0 - floor(whole/10.0)) < 0.05)
            {
                --digits_after;
                whole = floor(whole / 10.0 + 0.5);
            }
            if (digits_after < 0) digits_after = 0;

            std::stringstream ss;
            ss << std::fixed << std::setprecision(digits_after) << temporaryExpression.result;

            std::string stringResult = ss.str();
            //std::cout << "successfully converted " << temporaryExpression.result <<" long double to string" << std::endl;
            //std::cout << stringResult;
            // //std::cout << "size of expression: " << expressionString.size() << "Position of opening parenthesis: " << positionOfOpeningParenthesis << "Position of closing parenthesis: " << positionOfClosingParenthesis << std::endl;
            if(expressionString.size() <= positionOfOpeningParenthesis)
            {
             for (unsigned int index = expressionString.size(); index <= positionOfOpeningParenthesis+stringResult.size();index++)
             {
              expressionString+=' ';
             }
            }
            else
            {
             expressionString.replace(positionOfOpeningParenthesis,positionOfClosingParenthesis-positionOfOpeningParenthesis +1,stringResult);
            }
            //GETS RESULT AND PLACES IT BACK IN THE ORIGINAL EXPRESSION //
            // //std::cout << "successfully placed result of subexpression in string" << std::endl;
            // //std::cout << "NEW EXPRESSION: " << expressionString;
        }
      evaluate(); //Evaluates new expression containing the result of the sub expression
    }
}
int main() {

    std::cout << "To do list: " << std::endl;
    std::cout << "Validate equal bracket numbers" << std::endl;
    std::cout << "Add support for negative input numbers" << std::endl;
    while(true)
    {
     std::cout << "Please enter your expression: " ;
    std::string expression;
    std::cin >> expression;
    if(expression == "exit")
        break;
    Expression expressionToEvaluate(expression, false);
    std::cout << "RESULT: " << std::fixed <<  expressionToEvaluate.result<< std::endl;
    }

    return 0;
}

I suspect the code may be overly complex due to it being made up on the fly without any prior design planning or anything of the sort. I'd be interested in hearing what you guys have to say :)

share|improve this question
    
For anyone who intends to answer, I'm aware of the fact that the program doesn't check to see if the number of opening and closing parentheses are equal and that it doesn't support entering negative numbers –  Sky Lightna Jun 14 at 21:32
add comment

4 Answers 4

I suppose I may be somewhat biased, but if I were going to carry out this general task, I'd probably use a recursive descent parser. A simple version to handle +, -, * and / with the correct precedence can look something like this:

#include <iostream>
#include <string>
#include <cctype>

int expression();

char token() { 
    char ch;
    std::cin >> ch;
    return ch;
}

int factor() { 
    int val = 0;
    char ch = token();
    if (ch == '(') {
        val = expression();
        ch = token();
        if (ch != ')') {
            std::string error = std::string("Expected ')', got: ") + ch;
            throw std::runtime_error(error.c_str());
        }
    }
    else if (isdigit(ch)) {
        std::cin.unget();
        std::cin >> val;
    }
    else throw std::runtime_error("Unexpected character");
    return val;
}

int term() { 
    int ch;
    int val = factor();
    ch = token();
    if (ch == '*' || ch == '/') {
        int b = term();
        if (ch == '*')
            val *= b;
        else
            val /= b;
    }
    else std::cin.unget();
    return val;
}

int expression() {
    int val = term();
    char ch = token();
    if (ch == '-' || ch=='+') {
        int b = expression();
        if (ch == '+')
            val += b;
        else
            val -= b;
    }
    else std::cin.unget();
    return val;
}

int main(int argc, char **argv) {
    try {
        std::cout << expression();
    }
    catch(std::exception &e) {
        std::cout << e.what();
    }
    return 0;
}

Despite being less than one fifth the size, this already checks for balanced parentheses. Right now it's written to take data from std::cin, but getting it to read from an arbitrary stream (including a stringstream) would be trivial. Getting it to parse input from a string would be a little more work, but only a little.

The shunting-yard algorithm is also well known for this task. I don't say I see a huge advantage to it myself, but some (many?) people find it somewhat simpler. Depending on how fast function calls are on your particular CPU, it may be faster as well (but most modern CPUs make function calls pretty fast).

share|improve this answer
1  
Basically everything Jerry said. Though rather than write the recursive parser I would use tools to build it Lex/yacc –  Loki Astari Jun 15 at 16:16
    
@LokiAstari: I agree with your position, taking the comment you edited into your answer into account--for a somewhat larger language, I'd use lexer/parser tools, but for a grammar this small, I think they add more overhead than utility. –  Jerry Coffin Jun 15 at 18:36
add comment

There's a lot that can be reviewed here, but I'll mention some best-practices at a glance:

Keep your indentation and whitespace consistent! You seem to be primarily using four spaces for indentation, but you don't do this everywhere. What's even odd is that the body of evaluate() is not indented (unlike the other functions). One problem with this, especially it being a long function, is that readers may not know that it belongs to this function, and may even mistake it as global code! This can greatly reduce readability, especially with this amount of code.

<stdlib.h> and <math.h> are C libraries (ending in .h). In C++, they should respectively be <cstdlib> and <cmath>. As for the rest of the libraries, I'd sort them in some way (such as in alphabetically order) and check to see if there are any that are unused, since there are a lot of them.

operatorClass doesn't quite look concise. First of all, it should instead be named operator (Operator would be better, since it's a type). You also have everything public, while data members should always be private. The purpose of using classes (as opposed to structs) is to hide data. With data members as public, the state can be seen and changed from anywhere, which breaks encapsulation. I've seen that this properly done in Parenthesis, so it should be done here as well. You should also overload operator> since you've already overloaded operator<. If you define one, the user will expect the other to be usable as well. This also applies to operator== and operator!=.

The loop in main() could be simpler. One big problem with the loop termination is that the user isn't told what to type in order to exit. You could just have the user enter a "Y" or an "N" (be sure to account for uppercase or lowercase). An infinite loop is still okay here, though. Just be sure to provide smooth termination, otherwise there will be problems.

As for inside the loop, you should input into an std::string via std::getline(). This will also account for any spaces and newlines in the string. It is not effective enough to use std::cin since it will not format it properly.

Miscellaneous:

  • Instead of checking for a size greater than 0:

    operatorClassStack.size() > 0
    

    just use empty():

    !operatorClassStack.empty()
    

    (with the !, this basically means "not empty")

  • This should not be initialize with each function call:

    std::map<char, unsigned int> operatorClassMap;
    operatorClassMap['*']=2;
    operatorClassMap['/']=2;
    operatorClassMap['+']=1;
    operatorClassMap['-']=1;
    
  • If you compare something with true:

    if (someCondition == true)
    

    you can shorten it like this:

    if (someCondition)
    

    And if you're comparing something with false:

    if (someCondition == false)
    

    you can shorten it the same way with !:

    if (!someCondition)
    

    This example uses if, but this also works with while. The advantage of this is that you won't have to worry about a possible mismatch of == and =, which is quite common.

    It should be initialized only once, and it should also be const since it's not modified. If you have C++11, you can use an initializer list to accomplish this nicely.

share|improve this answer
1  
Wow! First timely and correct editing now a well thought out answer. Mad respect bro –  Sky Lightna Jun 14 at 23:26
1  
@SkyLightna: As mentioned, this was not intended to be a full review, but I currently don't have the means of tearing all this code apart. There are other C++ reviewers here who can help fill in the blanks, as well as addressing the overall design. –  Jamal Jun 14 at 23:28
    
Also, I'm not entirely sure if the operator overloading for the other operators is completely necessary because the < was overloaded only for use by the program itself, not the user. –  Sky Lightna Jun 14 at 23:33
    
@SkyLightna: I mentioned == and != as examples; you don't need to include them per se. It's still good to overload the other one for flexibility, but perhaps after the design has been fleshed out, you may not need either one. –  Jamal Jun 14 at 23:35
    
Fair enough, but what do you think about what I mentioned about the algorithm? Is it overcomplicated? –  Sky Lightna Jun 14 at 23:38
show 1 more comment

Personally I would use lex(flex) and yacc(bison) to do the same thing.

Edit:

After just doing it. I change my mind.
If all I was doing was an expression parser I would write a recursive descent parser like "Jerry Coffin" did. But if there was any chance that it would grow beyond just a simple expression parser (like having variables and state or any more complex functionality like functions) then I would use this technique.

Back to Description

These tools are a pain to learn when you are starting but are so darn useful when parsing stuff that I think it is a definite advantage to learn. The first time will take you a while but once you get used to them they become really easy.

I don't think its as fast as the hand written recursive descent parser. But it is so intuitive to read (if you have done CS at Uni) that the extra clarity makes up for a lot.

This is all it takes:

Lexer.l

Note: The definition of a number is slightly more complex than it could be (ie I could simplify it). This is because I ripped it straight out of a C parser. The definition for numbers is slightly more complicated in C (as leading 0 indicates an octal number (though I have removed the octal part from this example)).

But numbers support an optional leading '-' to indidicate negative numbers (though not as it stands a leading '+' (but that would not be hard to add)).

It also supports number using the standard exponent format ie 1.34e10.

%option c++
%option noyywrap

DIGIT           [0-9]
DIGIT1          [1-9]
INTNUM          {DIGIT1}{DIGIT}*
FRACT           "."{DIGIT}+
FLOAT           ({INTNUM}|0){FRACT}?
EXP             [eE][+-]?{DIGIT}+
NUMBER          -?{FLOAT}{EXP}?

WHITESPACE      [ \t\n]
%{
#define IN_LEXER
#include "parser.tab.h"
%}


%%

\+              {return '+';}
\-              {return '-';}
\*              {return '*';}
\/              {return '/';}
\(              {return '(';}
\)              {return ')';}
{NUMBER}        {return yy::Parser::token::NUMBER;}

{WHITESPACE}    {/*IGNORE*/}
.               {throw std::runtime_error("Invalid Character in Lexer");}

%%

Parser.y

Note: because of the way shift reduce parser work. The precedence of operators are implied by their position in the grammar. So '+' and '-' have the same precedence but have a lower precedence than '*' and '/'. So because the '(' and ')' are at the lowest level these operators have the highest precedence.

Also if precedence is the same the expression will be evaluated from left to right thus giving you the correct order you would expect from normal mathematics.

%skeleton "lalr1.cc"
%require "2.1a"
%define "parser_class_name" "Parser"

%{
// The type of the $$ value
#define YYSTYPE double

// Forward declarations used here
class FlexLexer;
int yylex(void*, FlexLexer& lexer);
double getNumber(FlexLexer& lexer);
%}


%parse-param {FlexLexer&          lexer}
%parse-param {double&             result}
%lex-param   {FlexLexer&          lexer}

%%

expression
    :    additive_expression                                 {result = $1;}

additive_expression
    :    multiplicative_expression                           {$$ = $1;}
    |    additive_expression '+' multiplicative_expression   {$$ = $1+$3;}
    |    additive_expression '-' multiplicative_expression   {$$ = $1-$3;}
    ;

multiplicative_expression
    :    primary_expression                                  {$$ = $1;}
    |    multiplicative_expression '*' primary_expression    {$$ = $1*$3;}
    |    multiplicative_expression '/' primary_expression    {$$ = $1/$3;}
    ;

primary_expression
    :    NUMBER                                              {$$ = getNumber(lexer);}
    |    '(' expression ')'                                  {$$ = $2;}
    ;


%%

Utility.cpp

#include "FlexLexer.h"
#include "parser.tab.h"
#include <sstream>

// The lexer calls this function
// To get the next token from the lexer.
// So all we do is ask the lexer for this token.
int yylex(void*, FlexLexer& lexer)
{
     return lexer.yylex();
}

// If the last value returned by the lexer (last token)
// is yy::Parser::token::NUMBER then the text of the input
// represents a number. So we ask the lexer for the text of
// the last token so we can covert it into a double.
double getNumber(FlexLexer& lexer)
{
    char const*     begin   = lexer.YYText();
    char const*     end     = begin + lexer.YYLeng();
    std::string     numberString(begin, end);

    double     result       = stod(numberString);

    return result;
}

// This function is called if there is an error in parsing the
// expression. You could just display the error message.
//
// This function takes an extra step and get the token that
// caused the error and incorporates it into the error message.
void yy::Parser::error(yy::location const&, std::string const& msg)
{
    std::string         lastToken(lexer.YYText(), lexer.YYText() + lexer.YYLeng());
    std::stringstream  extended;
    extended << msg << " -> Last Token: '" << lastToken << "' At line: " << lexer.lineno();

    throw std::runtime_error(extended.str());
}

main.cpp

#include "FlexLexer.h"
#include "parser.tab.h"

int main()
{
    // Get user input
    std::cout << "Expression:";
    std::string     line;
    std::getline(std::cin, line);

    // Set up lexer to read from a stream
    std::stringstream   lineStream(line);
    yyFlexLexer         lexer(&lineStream);

    // Set up parser tp read from lexer
    // and put output in result
    double              result;
    yy::Parser          parser(lexer, result);

    // Now do the parsing.
    parser.parse();

    std::cout << "Result: " << result << "\n";
}

Makefile

SRC     = lex.yy.cpp parser.tab.cpp utility.cpp main.cpp
OBJ     = $(patsubst %.cpp,%.o, $(SRC))
all:    exp

exp:    $(OBJ)
    g++ -o $@ $(OBJ)

%.o: %.cpp
    g++ -c $<

lex.yy.cpp: lexer.l parser.tab.cpp
    flex lexer.l
    mv lex.yy.cc lex.yy.cpp

parser.tab.cpp:  parser.y
    bison -d parser.y
    mv parser.tab.c parser.tab.cpp
share|improve this answer
    
LOL I havent done any CS, but I'll do my best to look at this :) –  Sky Lightna Jun 15 at 16:59
    
@SkyLightna: Have added the other files and a make file so you can see the whole application together. –  Loki Astari Jun 15 at 17:40
    
Ok thanks, bro. I really have no idea what on earth all of that is though, so I'll have to try and examine it and research a lot more –  Sky Lightna Jun 18 at 16:34
add comment

Tracing the code starting from main(), I didn't get very far until I noticed concerns.

Looking at your Expression constructor…

Expression::Expression(std::string expression, bool sub) //NOTE: being a sub means that it was surrounded
//by brackets '(' ')'
{


    hasBrackets = false;
    expressionString = expression;
    containsBrackets();
    // //std::cout << "Successfully complete 'containsBrackets' function" << std::endl;
    if (hasBrackets == true)
    {
     getParentheses();
     // //std::cout << "Successfully complete 'getParentheses()' function" << std::endl;
    getSubExpressions();
    // //std::cout << "Successfully complete 'getSubExpressions()' function" << std::endl;
    }
    evaluate();
    // //std::cout << "Successfully complete 'evaluate()' function" << std::endl;


}
  • Use initialization lists.
  • Your comment says, "being a sub means that it was surrounded by brackets '(' ')'". Why should that matter though? Indeed, you never even use the sub parameter.
  • hasBrackets = false; containsBrackets(); if (hasBrackets == true) { … } is cumbersome. Why not write if (hasBrackets = containsBrackets()) { … }? But even that is unsatisfactory…

Looking a containsBrackets() and getParentheses()

void Expression::containsBrackets()
{
    for(unsigned int index = 0; index < expressionString.size(); index++)
    {
        if (expressionString[index]=='(' ||expressionString[index]==')' )
        {
            hasBrackets = true;
        }
    }
}
void Expression::getParentheses() //Finds the parentheses and their positions in the expression
//so that their contents can be converted to sub expressions.
{
    for (unsigned int index = 0; index < expressionString.size(); index++)
    {
        if (expressionString[index] == '(' || expressionString[index] == ')')
        {
            Parenthesis temporary(index, expressionString[index]); //Stores the position and type of the parenthesis
            vector_parentheses.push_back(temporary);
        }
    }

    /* … */
}
  • The essentially do the same work. hasBrackets is essentially the same as !vector_parenthesis.empty().
  • In the Expression constructor, there is no need to treat the no-parentheses case specially. Iterating through an empty vector achieves the same effect.
  • Do you want to call them "brackets" or "parentheses"? Make up your mind!
share|improve this answer
    
Sorry, I've been trying to say 'parentheses' instead of 'brackets', but I'm still slipping :). Your suggestions are completely valid and I thank you for your contribution –  Sky Lightna Jun 15 at 17:01
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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