patterncppMinor
Creating polls and calculator with custom functions and without any global variable
Viewed 0 times
withoutglobalcreatingpollswithanycustomandfunctionsvariable
Problem
I solved this problem using custom functions and calling them in main function. The challenge was to avoid global variables. Please note that I'm C++ noob and this program is fairly large. I'm posting it here so experts can give me suggestion and point out my mistakes for good programming practices.
Is there any better code to improve calculator program function? For instance, a user enters 123+45*11/2 and it automatically solve the equation using
```
#include
#include
#include // for stringstream and eof()
//to avoid global variable declaration, I've used some parameters. Here I'm "Passing arguments by reference"
void startPoll(std::string &pollQuestion,
std::string &pollOption1,
std::string &pollOption2,
std::string &pollOption3);
void displayPoll(std::string &pollQuestion,
std::string &pollOption1,
std::string &pollOption2,
std::string &pollOption3,
int &pollVote,
int &plusOne,
int &plusTwo,
int &plusThree);
void acceptOnlyNumbers(std::string &pollVoteString,int &pollVote);
void printGraphBar (float &votePercentage);
void calculationFunction (double &result);
void checkAndCalculate (std::string &logicalOperator,
double &firstNumber,
double &secondNumber,
double &result);
int main (void)
{
std::string pollQuestion, pollOption1, pollOption2, pollOption3,
mainMenu;
int pollVote = 0, plusOne, plusTwo, plusThree,
selectMenu;
double result;
std::cout > number && verifyPoll.eof()) // To accept only numbers :)
{
break;
}
else
{
std::cout 0 && number > firstNumber;
std::cout > secondNumber;
std::cout > logicalOperator;
checkAndCalculate (logicalOperator, firstNumber, secondNumber, result);
}
void checkAndCalculate (std::string
Is there any better code to improve calculator program function? For instance, a user enters 123+45*11/2 and it automatically solve the equation using
std::stringstream?```
#include
#include
#include // for stringstream and eof()
//to avoid global variable declaration, I've used some parameters. Here I'm "Passing arguments by reference"
void startPoll(std::string &pollQuestion,
std::string &pollOption1,
std::string &pollOption2,
std::string &pollOption3);
void displayPoll(std::string &pollQuestion,
std::string &pollOption1,
std::string &pollOption2,
std::string &pollOption3,
int &pollVote,
int &plusOne,
int &plusTwo,
int &plusThree);
void acceptOnlyNumbers(std::string &pollVoteString,int &pollVote);
void printGraphBar (float &votePercentage);
void calculationFunction (double &result);
void checkAndCalculate (std::string &logicalOperator,
double &firstNumber,
double &secondNumber,
double &result);
int main (void)
{
std::string pollQuestion, pollOption1, pollOption2, pollOption3,
mainMenu;
int pollVote = 0, plusOne, plusTwo, plusThree,
selectMenu;
double result;
std::cout > number && verifyPoll.eof()) // To accept only numbers :)
{
break;
}
else
{
std::cout 0 && number > firstNumber;
std::cout > secondNumber;
std::cout > logicalOperator;
checkAndCalculate (logicalOperator, firstNumber, secondNumber, result);
}
void checkAndCalculate (std::string
Solution
-
Your code would greatly benefit from Object Oriented programming.
-
Having three options passed explicitly is ugly and it is difficult to maintain. Even if you are 100% sure you will always have three options, your could would benefit in treating them as a vector of strings:
so that you can iterate on them without repeating three times the same code. For two options this could be questionable, but starting from three I think there is no doubt about this. The same is true for the "plus" variables, they should be a single array.
-
In this snippet:
you are treating in a different way the first answer (with explicit
which takes care of printing the prompt, get the input, check it and repeat until a valid answer is given.
-
Here:
it seems more appropriate a signature like the following:
Your code would greatly benefit from Object Oriented programming.
Poll and PollOptions are good candidate to become a class.-
Having three options passed explicitly is ugly and it is difficult to maintain. Even if you are 100% sure you will always have three options, your could would benefit in treating them as a vector of strings:
typedef std::vector Options;so that you can iterate on them without repeating three times the same code. For two options this could be questionable, but starting from three I think there is no doubt about this. The same is true for the "plus" variables, they should be a single array.
-
In this snippet:
getline (std::cin, pollVoteString, '\n');
acceptOnlyNumbers(pollVoteString,pollVote);you are treating in a different way the first answer (with explicit
getline) and the following ones (inside the function acceptOnlyNumbers). You should modify the function to something like:int getNumber(std::string prompt);which takes care of printing the prompt, get the input, check it and repeat until a valid answer is given.
-
Here:
void checkAndCalculate (std::string &logicalOperator,
double &firstNumber,
double &secondNumber,
double &result)it seems more appropriate a signature like the following:
double checkAndCalculate (std::string operator,
double firstNumber,
double secondNumber);Code Snippets
typedef std::vector<std::string> Options;getline (std::cin, pollVoteString, '\n');
acceptOnlyNumbers(pollVoteString,pollVote);int getNumber(std::string prompt);void checkAndCalculate (std::string &logicalOperator,
double &firstNumber,
double &secondNumber,
double &result)double checkAndCalculate (std::string operator,
double firstNumber,
double secondNumber);Context
StackExchange Code Review Q#60290, answer score: 4
Revisions (0)
No revisions yet.