# Semi-simple calculator

I've written a simple calculator to train my C++ skills. Beware as this is my 1st project in C++ and it may contain some bad practices or something of the likes.

I want to know if my code follows good practices, is readable, moreless bug free and manages memory properly (I come from a Java background so that's been hard and sometimes confusing for me).

It supports addition, subtraction, multiplication and division.

Input can be:

• a single number (e.g. 2 returns 2)
• a single expression (e.g. 2+2 returns 4)
• a complex expression (e.g. 2+2+2 returns 6, 2+2*2/8 returns 1, 2+(2*2) / 6 * 3 + 2 returns 20)

I did not yet implement operator precedence and aren't even sure how to do it yet, but 2 + 2 * 2 = 8 (unless you write it as 2 + (2 * 2) - then it equals 6).

## main.cpp (main logic is here)

#include <cstdio>
#include <algorithm>
#include <memory>
#include <unordered_map>
#include <map>
#include <iostream>

#include "expressions/ConstExpression.h"
#include "expressions/CompoundExpression.h"
#include "operations/MultiplicationOperation.h"
#include "operations/SubtractionOperation.h"
#include "operations/DivisionOperation.h"

// single as in not interpreting nested parentheses
std::string interpret_single_paren(std::string::const_iterator &it) {
using std::size_t;
using std::string;

int level = 0;
string paren{};
while (true) {
if (*it == '(') {
if (level != 0)
paren += *it;
level += 1;
} else if (*it == ')') {
level -= 1;
if (level == 0)
break;
else
paren += *it;
} else {
paren += *it;
}
it++;
}

return paren;
}

std::shared_ptr<Expression> parse_number(std::string::const_iterator &it) {
std::string sNumber{};
while (std::isdigit(*it) || *it == '.') {
sNumber.push_back(*(it++));
}
it--;

float number = std::stof(sNumber);
return std::make_shared<ConstExpression>(number);
}

static std::map<char, Operation *> charToOperationMap{
{'-', &SubtractionOperation::getInstance()},
{'*', &MultiplicationOperation::getInstance()},
{'/', &DivisionOperation::getInstance()},
};

std::shared_ptr<Expression> parse_expr(const std::string &string) {
std::vector<std::shared_ptr<Expression>> expressions{};
std::shared_ptr<Expression> topExpression{nullptr};
Operation *cOperation = nullptr;

for (auto it = string.cbegin(); it != string.cend(); ++it) {
if (*it == ' ') continue;
else if (std::isdigit(*it) || *it == '.' /*as in .5*/) {
std::shared_ptr<Expression> expr = parse_number(it);
expressions.push_back(expr);
continue;
} else if (*it == '(') {
std::string paren = interpret_single_paren(it);
std::shared_ptr<Expression> expr = parse_expr(paren);
expressions.push_back(expr);
continue;
}

{
auto operation = charToOperationMap.find(*it);
if (operation != charToOperationMap.end()) {
if (cOperation != nullptr) {

// Optimization to allow for 2+2+2 to be created as CompoundExpr(2,2,2)
if (cOperation != operation->second) {
topExpression = std::make_shared<CompoundExpression>(expressions, *cOperation);
expressions.clear();
expressions.push_back(topExpression);
}
}
cOperation = operation->second;
continue;
}
}
}

if (cOperation != nullptr)
topExpression = std::make_shared<CompoundExpression>(expressions, *cOperation);
else if (expressions.size() == 1)
topExpression = std::shared_ptr<Expression>{expressions};
else
throw std::invalid_argument(
"Provided string is invalid (e.g.: 2 2 or -7 20, i.e. there's more than 1 number and no operators)");

}

int main() {
while (true) {
std::cout << "\nInput expression: ";
std::string line;
std::getline(std::cin, line);
if (line == "exit") break;
std::shared_ptr<Expression> expr = parse_expr(line);
std::cout << "Result of " << *expr << ": " << expr.get()->evaluate() << std::endl;
}

std::cout << "Bye" << std::endl;
return 0;
}


## Expression.h

#ifndef CALCULATOR_EXPRESSION_H
#define CALCULATOR_EXPRESSION_H

#include <ostream>

class Expression {
protected:
virtual std::ostream &output(std::ostream &os) const = 0;

public:
virtual float evaluate() const = 0;

friend std::ostream &operator<<(std::ostream &os, const Expression &expression) {
return expression.output(os);
}
};

#endif //CALCULATOR_EXPRESSION_H


## ConstExpression.h

#ifndef CALCULATOR_CONSTEXPRESSION_H
#define CALCULATOR_CONSTEXPRESSION_H

#include <ostream>
#include "Expression.h"

class ConstExpression : public Expression {
const float value;

protected:
std::ostream &output(std::ostream &os) const override {
return os << value;
}

public:
explicit ConstExpression(float value) : value(value) {}

inline float evaluate() const override {
return value;
}
};

#endif //CALCULATOR_CONSTEXPRESSION_H


## CompoundExpression.h

#ifndef CALCULATOR_COMPOUNDEXPRESSION_H
#define CALCULATOR_COMPOUNDEXPRESSION_H

#include <utility>
#include <vector>
#include <iterator>
#include <ostream>
#include "Expression.h"
#include "../operations/Operation.h"

class CompoundExpression : public Expression {
private:
const std::vector<std::shared_ptr<Expression>> exprs;
const Operation &operation;
protected:
std::ostream &output(std::ostream &os) const override {
for (size_t i = 0; i < exprs.size() - 1; ++i) {
os << *exprs[i] << " " << operation << " ";
}
os << *exprs[exprs.size() - 1];

return os;
}

public:
explicit CompoundExpression(std::vector<std::shared_ptr<Expression>> exprs, const Operation &operation) :
exprs(exprs), // exprs(std::vector<std::shared_ptr<Expression>>{exprs}),
operation(operation) {}

float evaluate() const override {
if (exprs.empty()) return 0;
float acc = exprs->evaluate();

for (size_t i = 1; i < exprs.size(); ++i) {
acc = operation.evaluate(acc, exprs[i]->evaluate());
}

return acc;
}
};

#endif //CALCULATOR_COMPOUNDEXPRESSION_H


## Operation.h

#ifndef CALCULATOR_OPERATION_H
#define CALCULATOR_OPERATION_H

#include <vector>
#include <ostream>

class Operation {
private:
static std::vector<Operation *> _operations;

public:
Operation() {
_operations.push_back(this);
}

virtual const std::string name() const = 0;

virtual float evaluate(float a, float b) const = 0;

friend std::ostream &operator<<(std::ostream &os, const Operation &operation);

static const std::vector<Operation *> operations() {
return _operations;
};
};

#endif //CALCULATOR_OPERATION_H


## Operation.cpp

#include "Operation.h"

std::vector<Operation*> Operation::_operations{};

std::ostream &operator<<(std::ostream &os, const Operation &operation) {
return os << operation.name();
}


#ifndef CALCULATOR_ADDITIONOPERATION_H

#include <ostream>
#include "Operation.h"

class AdditionOperation : public Operation {
private:

const std::string _name{"plus"};

public:

return operation;
}

void operator=(AdditionOperation const &)  = delete;

const std::string name() const override {
return _name;
}

float evaluate(float a, float b) const override {
return a + b;
}
};



## SubtractionOperation.h

#ifndef CALCULATOR_SUBTRACTIONOPERATION_H
#define CALCULATOR_SUBTRACTIONOPERATION_H

#include <ostream>
#include "Operation.h"

class SubtractionOperation : public Operation {
private:
SubtractionOperation() = default;

const std::string _name{"minus"};

public:
static SubtractionOperation &getInstance() {
static SubtractionOperation operation;

return operation;
}

SubtractionOperation(SubtractionOperation const &) = delete;

void operator=(SubtractionOperation const &)  = delete;

const std::string name() const override {
return _name;
}

float evaluate(float a, float b) const override {
return a - b;
}
};

#endif //CALCULATOR_SUBTRACTIONOPERATION_H


## MultiplicationOperation.h

#ifndef CALCULATOR_MULTIPLICATIONOPERATION_H
#define CALCULATOR_MULTIPLICATIONOPERATION_H

#include <ostream>
#include "Operation.h"

class MultiplicationOperation : public Operation {
private:
MultiplicationOperation() = default;
const std::string _name{"times"};

public:
static MultiplicationOperation &getInstance() {
static MultiplicationOperation operation;

return operation;
}

MultiplicationOperation(MultiplicationOperation const &) = delete;

void operator=(MultiplicationOperation const &)  = delete;

const std::string name() const override {
return _name;
}

float evaluate(float a, float b) const override {
return a * b;
}
};

#endif //CALCULATOR_MULTIPLICATIONOPERATION_H


## DivisionOperation.h

#ifndef CALCULATOR_DIVISIONOPERATION_H
#define CALCULATOR_DIVISIONOPERATION_H

#include <ostream>
#include "Operation.h"

class DivisionOperation : public Operation {
private:
DivisionOperation() = default;

const std::string _name{"divided by"};

public:
static DivisionOperation &getInstance() {
static DivisionOperation operation;

return operation;
}

DivisionOperation(DivisionOperation const &) = delete;

void operator=(DivisionOperation const &)  = delete;

const std::string name() const override {
return _name;
}

float evaluate(float a, float b) const override {
return a / b;
}
};

#endif //CALCULATOR_DIVISIONOPERATION_H

• @Incomputable I agree I've kind of got a lot of ifs in the interpret_single_paren function but other than that I can't see where else are there too many of them Dec 16, 2017 at 22:05
• I meant all of those classes and inheritance. Dec 16, 2017 at 22:39

3. I find your use of std::shared_ptr excessive. For example, in my opinion parse_number should rather return a std::unique_ptr because you're transferring ownership here. If you are looking for rules on when to use which smart pointer (or even a raw pointer), you should take a look at the Cpp Core Guidelines.
4. Please don't use std::endl. It doesn't only do what its name suggests, but also flushes the output buffer which is almost never necessary and can be harmful to performance. If you really need to flush the buffer, you should use flush() instead.
5. You actually have a missing include in CompoundExpression.h. You are using size_t (which really should be std::size_t as the global name is not guaranteed), but are missing an include for cstddef (or one of the other headers that define it).