2
\$\begingroup\$

For our first project in my C++ course this semester we had to do a simple calculator program. My background is mostly in Functional languages. What's the best way to do generic programming/functional style in C++?

We're doing C++14 standard in this course, but C++17 also welcome!

main.cpp

#include <iostream>
#include <fstream>
#include <iomanip>
#include <vector>
#include <memory>
#include <cmath>
#include <stack>
#include <algorithm>

//function declarations
bool isdouble(std::string tok);
std::deque<std::string> readFile(char* argv[]);


//definitions

std::deque<std::string> readFile(char* argv[]){
    // open the file for reading
    std::ifstream in(argv[1]);

    if (!in)
    {
        // Print an error and exit
        std::cerr << "File could not be opened for reading!" << std::endl;
        exit(1);
    }

    // vector to be read into
    std::deque<std::string> tokens;

    // read the file while we have input.
    while (in) {
        std::string buf;
        in >> buf;
        tokens.push_back(buf);
    }
    in.close();
    return tokens;
}
template<typename T>
class wrap{
public:
    T a;
    T b;
    std::string aString;
    std::string bString;
    wrap(T a,T b, std::string aString, std::string bString) :
            a(a),b(b),aString(aString),bString(bString){}

};


wrap<double> doublePop(std::stack<std::string> &stack){
    double a=std::stof(stack.top());
    auto aString = (stack.top());
    stack.pop();
    double b=std::stof(stack.top());
    auto bString = (stack.top());
    stack.pop();
    wrap<double> ret(a,b,aString,bString);
    return ret;
}

wrap<int> intPop(std::stack<std::string> &stack){
    int a=std::stoi(stack.top());
    auto aString = (stack.top());
    stack.pop();
    int b=std::stoi(stack.top());
    auto bString = (stack.top());
    stack.pop();
    wrap<int> ret(a,b,aString,bString);
    return ret;
}

bool isdouble(std::string tok){
    for(auto ch:tok){
        if(ch == '.'){
            return true;
        }
    }
    return false;
}

bool doublesCheck(std::stack<std::string> &stack){
    auto first = stack.top();
    stack.pop();
    auto second = stack.top();
    stack.push(first);
    return isdouble(first) || isdouble(second);
}

void calcsqrt(std::stack<std::string> &stack){
    auto operand = stack.top();
    stack.pop();
    double c;
    if(isdouble(operand)){
        c = sqrt(std::stod(operand));
    }else{
        c = sqrt(std::stoi(operand));
    }
    auto checkDoubleVal = std::to_string(c);
    if(isdouble(checkDoubleVal)){
        std::cout << "sqrt "<< operand << " = " << c << std::endl;
        stack.push(std::to_string(c));
    }else{
        int d = c;
        std::cout << "sqrt "<< operand << " = " << d << std::endl;
        stack.push(std::to_string(d));
    }


}

void add(std::stack<std::string> &stack){

    if(doublesCheck(stack)){
        auto wrap = doublePop(stack);
        auto c = std::to_string(wrap.a+wrap.b);
        stack.push(c);
        std::cout << wrap.aString << " + "<< wrap.bString << " = " << c << std::endl;
    }else {
        auto wrap = intPop(stack);
        auto c = std::to_string(wrap.a+wrap.b);
        stack.push(c);
        std::cout << wrap.aString << " + "<< wrap.bString << " = " << c << std::endl;
    }
}

void sub(std::stack<std::string> &stack){
    if(doublesCheck(stack)){
        auto wrap = doublePop(stack);
        auto c = std::to_string(wrap.a-wrap.b);
        stack.push(c);
        std::cout << wrap.aString << " - "<< wrap.bString << " = " << c << std::endl;
    }else {
        auto wrap = intPop(stack);
        auto c = std::to_string(wrap.a-wrap.b);
        stack.push(c);
        std::cout << wrap.aString << " - "<< wrap.bString << " = " << c << std::endl;
    }
}

void mult(std::stack<std::string> &stack){
    if(doublesCheck(stack)){
        auto wrap = doublePop(stack);
        auto c = std::to_string(wrap.a*wrap.b);
        stack.push(c);
        std::cout << wrap.aString << " * "<< wrap.bString << " = " << c << std::endl;
    }else {
        auto wrap = intPop(stack);
        auto c = std::to_string(wrap.a*wrap.b);
        stack.push(c);
        std::cout << wrap.aString << " * "<< wrap.bString << " = " << c << std::endl;
    }
}

void div(std::stack<std::string> &stack){
    if(doublesCheck(stack)){
        auto wrap = doublePop(stack);
        auto c = std::to_string(wrap.a/wrap.b);
        stack.push(c);
        std::cout << wrap.aString << " / "<< wrap.bString << " = " << c << std::endl;
    }else {
        auto wrap = intPop(stack);
        auto c = std::to_string(wrap.a/wrap.b);
        stack.push(c);
        std::cout << wrap.aString << " / "<< wrap.bString << " = " << c << std::endl;
    }
}

void reverse(std::stack<std::string> &stack){
    int times = std::stoi(stack.top());
    stack.pop();
    std::vector<std::string> buffer;

    while(times > 0){
        buffer.push_back(stack.top());
        stack.pop();
        times--;
    }

    for(auto item: buffer){
        stack.push(item);
    }

}

//main loop

int main(int argc, char* argv[]) {
    std::stack<std::string> stack;
    //read the file
    auto tokens = readFile(argv);
    //remove eof char
    tokens.pop_back();

    // setup the print out format for the precision required.
    std::cout.setf(std::ios::fixed,std::ios::floatfield);
    std::cout.precision(3);

    bool done = false;
    while(!done){
        std::deque<std::string> buffer;
        auto start = std::find(tokens.begin(),tokens.end(),"repeat");
        auto end = std::find(tokens.begin(),tokens.end(),"endrepeat");

        for(auto pos = start+1; pos != end; pos++){
            buffer.push_back(*pos);
        }

        for(int i = 0; i < 3; i++){
            tokens.insert(start,buffer.begin(),buffer.end());
        }
        start = std::find(tokens.begin(),tokens.end(),"repeat");
        end = std::find(tokens.begin(),tokens.end(),"endrepeat");
        tokens.erase(start,end+1);

//end process if all repeats processed
        if(std::find(tokens.begin(),tokens.end(),"repeat") == tokens.end()){
            done = true;
        }

    //process the tokens
    for(std::string tok : tokens){
        auto check = std::isdigit(static_cast<unsigned char>(tok[0]));
        //if token is a number
        if(check){
            stack.push(tok);
        }
        else{
            //empty check
            if(stack.empty()){
                std::cout << "not enough operands for this operation" << std::endl;
                break;
            }
            if(tok == "add"){
                add(stack);
                continue;
            }
            if(tok == "sub"){
                sub(stack);
                continue;
            }
            if(tok == "mult"){
                mult(stack);
                continue;
            }
            if(tok == "div"){
                div(stack);
                continue;
            }
            if(tok == "pop"){
                stack.pop();
                continue;
            }
            if(tok == "sqrt"){
                calcsqrt(stack);
                continue;
            }
            if(tok == "reverse"){
                reverse(stack);
                continue;
            }
            if(tok == "repeat"){
                continue;
            }
            if(tok == "endrepeat"){
                continue;
            }
            std::cout << "invalid input, program exiting" << std::endl;
            exit(1);
        }
    }
    return 0;
}
\$\endgroup\$
3
  • 2
    \$\begingroup\$ Please comment on the specification / requirements your program must satisfy. You go to some trouble to support both double and int, despite the fact that small int values are readily accommodated by double. There's many lines of copy-n-paste that would disappear if you could have the + - * / operators passing around a suitable abstraction. \$\endgroup\$
    – J_H
    Aug 16, 2017 at 14:54
  • \$\begingroup\$ Doesn't compile - you've missed a } somewhere... \$\endgroup\$ Aug 16, 2017 at 16:46
  • \$\begingroup\$ sorry I forgot to include the specs. We we're supposed to handle ints and doubles but not promote unless necessary. I will be sure to include specs next time in code review. Thanks! \$\endgroup\$ Aug 17, 2017 at 3:31

1 Answer 1

3
\$\begingroup\$

Headers

The code is missing <deque>; conversely, <iomanip> and <memory> are both included but never used.

The code includes <cmath> but then uses unqualified sqrt() as if <math.h> were included instead. I recommend changing the code to call std::sqrt().

Repetition of string literals

For tokens such as "repeat", it's easy to make a typo that will result in run-time bugs. Far better to catch these at compile time, by using named constants to refer to them.

Argument parsing

The main() function passes argv but not argc to readFile(). This deprives readFile() of the information it needs to determine whether it's safe to access argv[1].

I'd write the main() something like

int main(int argc, char* argv[])
{
    for (auto i = 1u;  i < argc;  ++i) {
        auto result = processFile(argv[i]);
        if (result)
            // non-zero means error
            return result;
    }
}

and declare processFile() something like

// @brief Read and evaluate the file, writing results to standard output.
// @param filename the name of the file to read
// @return zero on success, or error code on failure
int processFile(const char *filename);

In passing, I'll also mention that parseFile() is very rude to exit() when it fails - make it return an error value, so that the caller can decide whether to give up, or move to the next input file, or to print a friendly message, and ask an operator for a substitute filename.

Type checking in operators

The pattern shown amongst the operators will become difficult to maintain. Imagine you decide to add rational numbers to your calculator; now you need to edit each operator, changing

    if (doublesCheck(stack)) {

    } else {

    }

to

    if (doublesCheck(stack)) {

    } else if(isRational(stack)) {

    } else {

    }

It's easy to miss one! And the problem is repeated when you later introduce complex numbers to the mix.

You can invert this pattern of control using inheritance and interfaces (better known in C++ by their mechanism - abstract base classes). The simple case is the functions that modify a single argument, such as square root:

class AbstractNumber {
    // replace this number with its square root
    virtual void do_sqrt() = 0;
};

class FloatingPointNumber {
    double value;
    void do_sqrt() override
    {
        value = std::sqrt(value);
    }
};

Now, when you introduce a new number type, the changes are together, and the compiler can let you know if you forget to override one of the pure virtual methods. Similarly, if you declare a new method, the compiler will tell you where you forgot to provide an implementation.

It gets harder with binary operations, such as addition and multiplication; you then encounter the "double despatch" problem. In this case, you can at least temporarily circumvent that by having the argument pairs coerce to the nearest compatible types.

\$\endgroup\$
2
  • \$\begingroup\$ Thanks a lot for taking the time to go over this. I had never even heard of named constants before you mentioned them. I never understood what the virtual keyword meant before you used it here, will do some reading. \$\endgroup\$ Aug 17, 2017 at 3:34
  • \$\begingroup\$ By "named constant" I just mean something like static auto const keyword_repeat = "repeat"; and the like - it's not complex at all technically, but helps avoid a whole class of trivial error. \$\endgroup\$ Aug 17, 2017 at 7:53

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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