String Calculator

Question

This is a calculator I made for fun and also to practice a bit. My goal was to make a calculator that can handle user input as well as a scientific calculator.
I made it as a Singleton to keep things tidy and provide some additional calculator functionality.

#include <iostream>
#include <vector>
#include <utility>
#include <string>
#include <cstring>
#include <cmath>
static const long double pi_num = 3.1415926535897932;
template <typename T, typename U>
static T factorial(U num)
{
 T res = 1;
 while (num > 1)
 {
 res *= num;
 --num;
 }
 return res;
}
// singleton
template <typename NUM_TYPE>
class calculator
{
public:
 static calculator &get();
 calculator(const calculator &) = delete;
 calculator &operator=(const calculator &) = delete;
 static NUM_TYPE calc(const std::string &expression);
 static NUM_TYPE calc(const char *expression);
 NUM_TYPE calc_substr(const std::string &, unsigned begin, unsigned end);
 static const std::string output();
 static void printOutput();
 static bool error();
 static NUM_TYPE ans();
private:
 calculator() {}
 std::string error_msg;
 NUM_TYPE answer = 0;
 bool error_flag = false;
 bool paren_flag = false; // for preventing parentheses from overwriting answer
 static void applyFunction(std::string &, NUM_TYPE &);
};
template <typename NUM_TYPE>
calculator<NUM_TYPE> &calculator<NUM_TYPE>::get()
{
 static calculator<NUM_TYPE> Calculator;
 return Calculator;
}
template <typename NUM_TYPE>
NUM_TYPE calculator<NUM_TYPE>::calc(const std::string &expression)
{
 return get().calc_substr(expression, 0, expression.length() - 1);
}
template <typename NUM_TYPE>
NUM_TYPE calculator<NUM_TYPE>::calc(const char *expression)
{
 return get().calc_substr(expression, 0, strlen(expression) - 1);
}
template <typename NUM_TYPE>
NUM_TYPE calculator<NUM_TYPE>::calc_substr(const std::string &expression, unsigned begin, unsigned end)
{
 // the calculator splits the input into segments (units) each containing an operation and a number
 // these segments (units) are stored in calc_units
 std::vector< std::pair<char, NUM_TYPE> > calc_units;
 std::string function;
 function.reserve(6);
 NUM_TYPE num = 0, res = 0;
 char operation = '+';
 bool operation_flag = true; // setting the operation flag to true since
 // the first number's plus sign is usually omitted
 bool negative_flag = false;
 bool function_flag = false;
 error_flag = false;
 // parsing the string and calculating functions
 for (int i = begin; i <= end; ++i)
 {
 if (expression[i] == '+' || expression[i] == '-' || expression[i] == '*' ||
 expression[i] == '/' || expression[i] == '%' || expression[i] == '^')
 {
 if (operation_flag)
 {
 if (expression[i] == '-') // negative number
 negative_flag = true;
 else if (operation == '*' && expression[i] == '*') // python notation for exponentiation
 operation = '^';
 else
 {
 error_flag = true;
 error_msg = "Syntax Error";
 return 0;
 }
 }
 else if (function_flag)
 {
 error_flag = true;
 error_msg = "Syntax Error";
 return 0;
 }
 else
 {
 operation = expression[i];
 operation_flag = true;
 negative_flag = false;
 }
 }
 else if (expression[i] == '!')
 calc_units[calc_units.size() - 1].second =
 factorial<NUM_TYPE>(calc_units[calc_units.size() - 1].second);
 else if (expression[i] >= 'a' && expression[i] <= 'z')
 {
 function.clear();
 while ((expression[i] >= 'a' && expression[i] <= 'z') && i <= end)
 {
 function.push_back(expression[i]);
 ++i;
 }
 i--;
 if (function == "ans")
 {
 num = answer;
 if (negative_flag)
 num *= -1;
 if (operation_flag == false) // omitting the '*' in multiplication
 operation = '*';
 calc_units.push_back(std::make_pair(operation, num));
 num = 0;
 operation_flag = false;
 negative_flag = false;
 }
 else if (function == "pi")
 {
 num = pi_num;
 if (negative_flag)
 num *= -1;
 if (operation_flag == false) // omitting the '*' in multiplication
 operation = '*';
 calc_units.push_back(std::make_pair(operation, num));
 num = 0;
 operation_flag = false;
 negative_flag = false;
 }
 else
 function_flag = true;
 }
 // parsing numbers and applying functions
 // the user might use a decimal point without a zero before it to show a number smaller than one
 // example: 1337 * .42 where the zero in 0.42 is omitted
 else if ((expression[i] >= '0' && expression[i] <= '9') || expression[i] == '.')
 {
 while (expression[i] >= '0' && expression[i] <= '9' && i <= end)
 {
 num = 10 * num + (expression[i] - '0');
 ++i;
 }
 if (expression[i] == '.') // decimal point
 {
 ++i;
 unsigned decimals_count = 0;
 NUM_TYPE decimals = 0;
 while (expression[i] >= '0' && expression[i] <= '9' && i <= end)
 {
 decimals = 10 * decimals + (expression[i] - '0');
 decimals_count++;
 ++i;
 }
 num += decimals / pow(10, decimals_count);
 decimals = 0;
 decimals_count = 0;
 }
 if (negative_flag) // negative number
 num *= -1;
 // applying functions
 if (function_flag)
 {
 applyFunction(function, num);
 if (error_flag)
 {
 error_msg = "Unknown Function";
 return 0;
 }
 function_flag = false;
 }
 if (operation_flag == false) // omitting the '*' in multiplication
 operation = '*';
 calc_units.push_back(std::make_pair(operation, num));
 num = 0;
 operation_flag = false;
 negative_flag = false;
 --i;
 }
 else if (expression[i] == '(')
 {
 unsigned open = ++i;
 // the user might open parentheses but not close them
 // in the case that several parentheses are opened but only some of them
 // are closed, we must pair the closest open and close parentheses together
 // parenthesis_count is used to check if a close parenthesis belongs to
 // the current open paranthesis
 int parenthesis_count = 1;
 while (parenthesis_count > 0 && i <= end)
 {
 if (expression[i] == '(')
 ++parenthesis_count;
 if (expression[i] == ')')
 --parenthesis_count;
 ++i;
 }
 i--;
 paren_flag = true; // preventing parentheses from overwriting answer
 num = get().calc_substr(expression, open, i);
 if (error_flag)
 return 0;
 if (negative_flag)
 num *= -1;
 // applying functions
 if (function_flag)
 {
 applyFunction(function, num);
 if (error_flag)
 {
 error_msg = "Unknown Function";
 return 0;
 }
 function_flag = false;
 }
 if (operation_flag == false) // omitting the '*' in multiplication
 operation = '*';
 calc_units.push_back(std::make_pair(operation, num));
 num = 0;
 operation_flag = false;
 negative_flag = false;
 paren_flag = false;
 }
 }
 for (int i = 0; i < calc_units.size(); ++i)
 {
 if (calc_units[i].first == '+')
 {
 num = calc_units[i].second;
 }
 else if (calc_units[i].first == '-')
 {
 num = calc_units[i].second * -1;
 }
 // left-to-right associativity
 else if (calc_units[i].first == '*' || calc_units[i].first == '/')
 {
 res -= num;
 while (i < calc_units.size() && (calc_units[i].first == '*' || calc_units[i].first == '/'))
 {
 if (calc_units[i].first == '*')
 num *= calc_units[i].second;
 else if (calc_units[i].first == '/')
 {
 if (calc_units[i].second == 0)
 {
 error_flag = true;
 error_msg = "Math Error";
 return 0;
 }
 else
 num /= calc_units[i].second;
 }
 ++i;
 }
 --i;
 }
 // right-to-left associativity
 else if (calc_units[i].first == '^' || calc_units[i].second == '%')
 {
 res -= num;
 NUM_TYPE temp;
 int count = 0;
 // finding where the operations with right-to-left associativity end
 while (i + count + 1 < calc_units.size() && (calc_units[i + count + 1].first == '^' ||
 calc_units[i + count + 1].first == '%'))
 ++count;
 temp = calc_units[i + count].second;
 for (int j = count; j >= 0; --j)
 {
 if (calc_units[i + j].first == '^')
 temp = pow(calc_units[i + j - 1].second, temp);
 if (calc_units[i + j].first == '%')
 temp = (long long) calc_units[i + j - 1].second % (long long) temp;
 }
 if (calc_units[i - 1].first == '+')
 num = temp;
 else if (calc_units[i - 1].first == '-')
 num = temp * -1;
 else if (calc_units[i - 1].first == '*')
 {
 num /= calc_units[i - 1].second;
 num *= temp;
 }
 else if (calc_units[i - 1].first == '/')
 {
 num *= calc_units[i - 1].second;
 num /= temp;
 }
 i += count;
 }
 res += num;
 }
 if (paren_flag == false) // preventing parentheses from overwriting answer
 answer = res;
 return res;
}
template <typename NUM_TYPE>
const std::string calculator<NUM_TYPE>::output()
{
 if (get().error_flag)
 return get().error_msg;
 else
 {
 using std::to_string; // for compatibility with non-fundamental data types
 return to_string(get().answer);
 }
}
template <typename NUM_TYPE>
void calculator<NUM_TYPE>::printOutput()
{
 if (get().error_flag)
 std::cout << get().error_msg;
 else
 std::cout << get().answer;
}
template <typename NUM_TYPE>
bool calculator<NUM_TYPE>::error()
{
 return get().error_flag;
}
template <typename NUM_TYPE>
NUM_TYPE calculator<NUM_TYPE>::ans()
{
 return get().answer;
}
template <typename NUM_TYPE>
void calculator<NUM_TYPE>::applyFunction(std::string &function, NUM_TYPE &num)
{
 if (function == "abs")
 num = fabs(num);
 else if (function == "sqrt")
 num = sqrt(num);
 else if (function == "cbrt")
 num = cbrt(num);
 else if (function == "sin")
 num = sin(num);
 else if (function == "cos")
 num = cos(num);
 else if (function == "tan")
 num = tan(num);
 else if (function == "cot")
 num = 1 / tan(num);
 else if (function == "sec")
 num = 1 / cos(num);
 else if (function == "csc")
 num = 1 / sin(num);
 else if (function == "arctan")
 num = atan(num);
 else if (function == "arcsin")
 num = asin(num);
 else if (function == "arccos")
 num = acos(num);
 else if (function == "arccot")
 num = atan(1 / num);
 else if (function == "arcsec")
 num = acos(1 / num);
 else if (function == "arccsc")
 num = asin(1 / num);
 else if (function == "sinh")
 num = sinh(num);
 else if (function == "cosh")
 num = cosh(num);
 else if (function == "tanh")
 num = tanh(num);
 else if (function == "coth")
 num = 1 / tanh(num);
 else if (function == "sech")
 num = 1 / cosh(num);
 else if (function == "csch")
 num = 1 / sinh(num);
 else if (function == "arctanh")
 num = atanh(num);
 else if (function == "arcsinh")
 num = asinh(num);
 else if (function == "arccosh")
 num = acosh(num);
 else if (function == "arccoth")
 num = atanh(1 / num);
 else if (function == "arcsech")
 num = acosh(1 / num);
 else if (function == "arccsch")
 num = asinh(1 / num);
 else if (function == "log")
 num = log10(num);
 else if (function == "ln")
 num = log(num);
 else if (function == "exp")
 num = exp(num);
 else if (function == "gamma")
 num = tgamma(num);
 else if (function == "erf")
 num = erf(num);
 else
 get().error_flag = true;
 function.clear();
}

Possible way of using the calculator:

using Calculator = calculator<long double>;
int main()
{
 std::string expression;
 while (true)
 {
 std::getline(std::cin, expression);
 Calculator::calc(expression);
 if (Calculator::error())
 std::cout << Calculator::output() << "\n\n";
 else
 std::cout << "= " << std::setprecision(15) << Calculator::ans() << "\n\n";
 }
}

Output example:

4400 * 1337 - 42 / 7 +たす 9000
=わ 5891794
2sin(pi/4)cos(pi/4)
= 1
ans * 32
= 32
2 * 2 ^ 2 ^ 3
= 512
(2 + 3) * 4
= 20
5(8+9)
= 85
2 * -4
= -8
tan(2)*log(5)/exp(6)
= -0.00378574198801152
sin1sqrt2
= 1.19001967905877
1 / 0
Math Error
sin*cos
Syntax Error
2 */ 4
Syntax Error
lol(1234)
Unknown Function

A few questions:

• Is my extensive use of flags causing code-spaghetti?
• Does my code need more comments?
• Was it a good idea to use the Singleton design pattern?

Let me know what you think! Suggestions and Ideas are very welcome :)

• L. F.: Welcome to Code Review! You can take the tour and visit our FAQs to further familiarize yourself with our community.

Answer 1

Use the constant M_PI (and others) from <cmath> instead of defining your own.

Singletons are bad, and there is no need for one here. I would recommend you avoid this pattern.

There is no way to cleanly exit the program.

Break out some functions, the body of the main calculation function is too long to be easy to understand.

Use std::stringstream and it's formatted input functions to read numbers etc instead of writing your own code for this.

You should use the correct algorithm for parsing mathematical expressions: shunting yard algorithm.

Regarding more or less comments. Your code should be structured such that comments are not necessary. Break out functions wherever you think you need a comment and make the function name sat what what your comment would have had is one way to think of it. Of course it's not always possible but it's one way to think about it.

Eg. Instead of having:

// Read in a number from string
... Lots of code...

Do:

auto number = read_number(input_string);

If you apply this consistently you'll find that you get more readable and maintainable code with less comments.

I'm missing unit tests, this is an obvious class to test with unit testing to make sure it works and produces the correct result.

I'm going to stop here without going too deep into the technical issues with the code such as using int instead of vector<>::size_type etc because I believe that you have bigger things to address (e.g. use the right algorithm and test your code)

• Saleh Shamloo: Thank you for your in-depth review! I do agree that I should've used more functions. The main function body is kind of a mess right now... std::stringstream looks very interesting. It would've saved me a lot of headaches. Does shunting yard algorithm address input like 2sinxcosx too? I did consider converting my input into RPN for parsing, but I was concerned about omitted multiplication signs and parantheses. It was a good exercise too, so I decided to write my own algorithm (coming to think of it, it's very similar to RPN).
• Saleh Shamloo: I did use something like unit tests (much less rigorous) when writing my code. I should get into the habit of writing proper unit tests though...
• N. Shead: Worth noting that M_PI isn't standard C++. From C++20 there is std::numbers::pi, though, I believe.