Hello Ineff,
Well
jonnin
seems to have gotten farther than I have and makes very good points.
So far I have worked on the "DoCalc.h" file and this is what I see.
Try to avoid using the
#pragma once
. I believe not everyone can use it. A better choice is header guards that I will show later.
The header files:
1 2
|
#include <string>
#include <iostream>
|
In this program these should be in "main".
using namespace std;
should never be put or needed in a header file. Read this
http://www.lonecpluspluscoder.com/2012/09/22/i-dont-want-to-see-another-using-namespace-xxx-in-a-header-file-ever-again/
A note on using {}s. Whichever method you choose be consistent in the use. It really breaks up the rhythm when reading and trying to keep track of the {}s. Of the choices I prefer the "Allman" style
https://en.wikipedia.org/wiki/Indentation_style It seems to be the easiest to read and work with, at least for me. In the end it it your choice and what you are comfortable with.
When it comes to classes it is more common to put the class in a header file and the functions in a ".cpp" file. What you have now works, but keep this in mind for the future.
In the class putting everything under "public:" works, but in a sense it kind of defeats the purpose of keeping the variables protected form code anywhere in the program from changing them.
When I gave the program its first run it showed me this:
Press
1 to perform Calculations
2 for Guess the Number
3 Exit
Enter choice: 1
Add, Subtract, Multiply, Divide
|
First it gives you an idea of how changing the first prompt looks. But when the last line came up I was not sure what needed to be entered.
That line
Add, Subtract, Multiply, Divide
is nice, but you are expecting the user to know how to spell correctly and counting on it. And what happens if the user puts "sub" for subtract. It will not work. Now trying to think of every possibility of what to check for you are bound to miss something and that is what someone will use to break the program. A numbered menu keeps it simple and easy to use. Best to keep it simple if you can.
Again a numbered menu would work better. This
std::cout << "\n 1. Add\n 2. Subtract\n 3. Multiply\n 4. Divide\n Enter choice: ";
gave the following output on the screen:
Press
1 to perform Calculations
2 for Guess the Number
3 Exit
Enter choice: 1
1. Add
2. Subtract
3. Multiply
4. Divide
Enter choice:
|
To this I would add a fifth choice to exit. Notice how the end of the string is
choice: "
. The space after the ":" alone with dropping the "std::endl", which puts the "std::cin" at the end of the line and not the next line, makes for a better prompt IMHO.
Also that would mean changing
std::string input;
to
size_t choice;
. "size_t" AKA "unsigned int" because you should be working positive numbers only, but an "int" will be just fine and should you allow negative numbers the "int" is a better choice.
In the all four functions
int x, y, sum;
, but "x" and "y" tend to make me thing of points on a graph. A better possible choice could be
int num1{}, num2{}, result{};
The {}s, available form C++11 on, initialize the variables to (0) zero. Not always necessary, but does give peace of mind that they do not contain a garbage value. If you are not doing addition "sum" is a bit misleading.
In your division function you have defined your variables as "int"s which means that you are doing integer division. This may not always produce the results that you need. Example: 1 / 4 = 0.025, but storing that in an int would give you (0) zero because it would store the whole number and drop the decimal part. Then you are asking why it prints (0) zero.
Another thing you should do is check that "num2" is greater then (0) zero otherwise you will get the run time error "divide by 0" and your program would stop.
This is what"DoCal.h" could look like:
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163
|
#ifndef DOCALC_H
#define DOCALC_H
class DoCalc
{
std::string input;
bool valid = true;
bool again = true;
public:
int add()
{
int x, y, sum;
std::cout << "\n Enter first number to add" << std::endl;
std::cin >> x;
//std::cout << std::endl; // <--- Start the next line with "\n".
std::cout << "\n Enter second number to add" << std::endl;
std::cin >> y;
sum = x + y;
std::cout << "\n Your answer is " << sum << std::endl;
valid = false;
return sum;
}
int subtract()
{
int x, y, sum;
std::cout << "Enter first number to subtract" << std::endl;
std::cin >> x;
std::cout << std::endl;
std::cout << "Enter second number to subtract" << std::endl;
std::cin >> y;
sum = x - y;
std::cout << "Your answer is " << sum << std::endl;
valid = false;
return sum;
}
int multiply()
{
int x, y, sum;
std::cout << "Enter first number to multiply" << std::endl;
std::cin >> x;
std::cout << std::endl;
std::cout << "Enter second number to multiply" << std::endl;
std::cin >> y;
sum = x * y;
std::cout << "Your answer is " << sum << std::endl;
valid = false;
return sum;
}
int divide()
{
int x, y, sum; // <--- Would work better as doubles.
std::cout << "Enter first number to divide" << std::endl;
std::cin >> x;
std::cout << std::endl;
std::cout << "Enter second number to divide" << std::endl;
std::cin >> y;
// <--- Check for greater than (0) zero or "!=" (0) zero.
sum = x / y;
std::cout << "Your answer is " << sum << std::endl;
valid = false;
return sum;
}
void DoCalculation()
{
do
{
//std::cout << "Add, Subtract, Multiply, Divide" << std::endl;
std::cout << "\n 1. Add\n 2. Subtract\n 3. Multiply\n 4. Divide\n Enter choice: ";
std::cin >> input;
for (std::string::iterator i = input.begin(); i < input.end(); i++)
{
*i = std::tolower(*i);
}
// <--- An alternative, but neither should be used.
//for (size_t i = 0; i < input.size(); i++)
//{
// std::tolower(input[i]);
//}
if (input == "add")
{
add();
}
else if (input == "subtract")
{
subtract();
}
else if (input == "multiply")
{
multiply();
}
else if (input == "divide")
{
divide();
}
else
{
std::cout << "Please check spelling and try again" << std::endl;
}
do
{
std::cout << "Would you like to perform another calculation?" << std::endl;
std::cout << "Yes or No" << std::endl;
std::cin >> input;
for (std::string::iterator i = input.begin(); i < input.end(); i++)
{
*i = std::tolower(*i);
}
if (input == "yes")
{
DoCalc();
}
else if (input == "no")
{
valid = false;
again = false;
}
else
{
std::cout << "Please enter either Yes or No" << std::endl;
}
} while (again == true); // End 2nd do/while.
} while (valid == true); // End 1st do/while
} // End function DoCalculation()
};
#endif // !DOCALC_H
|
Lines 1,2 and 163 are the header guard I mentioned earlier.
"public:" is moved down to line 10 making the variables "private" because a class is "private" by default. The rest of the code has comments you should read.
The for loop to change case, which you really do not need, shows a simpler way of doing what you did. You do not need to be as fancy and I think what you have shoold work.
I know that this may not be what you are expecting, but as a whole it all goes together.
Almost forgot. Notice the blank lines to break up the code. it makes it much easier to read. The compiler does not care about white space, blank lines or comments, but someone reading, including your-self, it does make a difference.
Hope that helps,
Andy