patterncppMinor
Summation calculator of integers, squares, and cubes
Viewed 0 times
cubesandsquaressummationintegerscalculator
Problem
Code works exactly how I want it to, but any simplification to the code that could be made would be much appreciated (granted this program uses almost everything within the scope of my C++ knowledge).
// sum calculator
#include
#include
//integer sum: 0 + 1 + 2 + ... + n
int integer_sum(int a, int b){
int int_sum(0);
while(a > choice;
std::string restraint("First value must be less than the second");
std::string request_range_1("First Range value: ");
std::string request_range_2("Second Range value: ");
int range_1;
int range_2;
switch(choice){
case 'd':
break;
case 'a':
std::cout > range_1;
std::cout > range_2;
std::cout > range_1;
std::cout > range_2;
std::cout > range_1;
std::cout > range_2;
std::cout << std::endl << "The sum of the cubes in the range: " << "[" << range_1 << "," << range_2 << "]\nis: ";
std::cout << cube_sum(range_1, range_2) << std::endl << std::endl;
break;
default:
std::cout << "Incorrect operation \n\n";
break;
}
if(choice == 'd')
break;
}
std::cout << "Calculator OFF" << std::endl;
}Solution
Refactor IO
In each case of your switch statement, you have this at the very beginning:
Rather than re-writing this out for each case, you should refactor this to its own function that returns the two inputs. That way, you can just call that function to get the inputs.
Also, speaking of inputs...
Never trust the user!
When you are grabbing the two input numbers for an operation, you show the user a restraint message that says that the first number must be smaller than the second. While your function technically do "handle" this by returning 0, it would be good practice to not call those functions at all and to instead print out to the user if they accidentally (or purposely) entered a bigger number first.
You could include this in the extracted input-getting function described above. You can even create a loop inside this that repeatedly asks the user for proper input before actually returning the input.
early exit -- early exit -- early exit -- early exit....
This is a bit of code I grabbed from the end of the switch statement. Note that the only time that this bit of code is going to be reached is if the user hit
Therefore, you can do some simplifying here. Rather than exiting from the switch statement only to do another conditional on whether or not to break the loop, just return from the entire program as soon as it is discovered that the user entered 'd':
This is much simpler and removes the redundancy from your code.
More refactoring and simplification
Your three
and
and
The rest of the code in the function is identical (of course, except for the variable names). This means that we can do some refactoring.
To simplify the code, let's write a blank function that follows the same format as the others:
Notice the last parameter: this will allow the function to know whether to put
Now, the body of the function looks like this:
Note, I made two other changes:
Now, instead of having three functions, you can just have this one function and call it with different
BUT WAIT!! You can make your program even more flexible now! Rather than having the user enter the "type" of exponent series thing they want, have them input these three things:
That way, you won't even need a switch statement at all! The user could enter numbers like
In each case of your switch statement, you have this at the very beginning:
std::cout > range_1;
std::cout > range_2;Rather than re-writing this out for each case, you should refactor this to its own function that returns the two inputs. That way, you can just call that function to get the inputs.
Also, speaking of inputs...
Never trust the user!
When you are grabbing the two input numbers for an operation, you show the user a restraint message that says that the first number must be smaller than the second. While your function technically do "handle" this by returning 0, it would be good practice to not call those functions at all and to instead print out to the user if they accidentally (or purposely) entered a bigger number first.
You could include this in the extracted input-getting function described above. You can even create a loop inside this that repeatedly asks the user for proper input before actually returning the input.
early exit -- early exit -- early exit -- early exit....
if(choice == 'd')
break;
}
std::cout << "Calculator OFF" << std::endl;This is a bit of code I grabbed from the end of the switch statement. Note that the only time that this bit of code is going to be reached is if the user hit
d as their input.Therefore, you can do some simplifying here. Rather than exiting from the switch statement only to do another conditional on whether or not to break the loop, just return from the entire program as soon as it is discovered that the user entered 'd':
case 'd':
std::cout << "Calculator OFF" << std::endl;
return 0;This is much simpler and removes the redundancy from your code.
More refactoring and simplification
Your three
_sum functions are all very similar except for a single line. These lines:int_sum += a;and
squ_sum += a*a;and
cub_sum += a*a*a;The rest of the code in the function is identical (of course, except for the variable names). This means that we can do some refactoring.
To simplify the code, let's write a blank function that follows the same format as the others:
int sum_exponent_series(int a, int b, int exp);Notice the last parameter: this will allow the function to know whether to put
a, aa, or aa*a... Which, in fact, can be simplified using 's pow.Now, the body of the function looks like this:
int sum(0);
for(; a <= b; a++) {
sum += pow(a, exp);
}
return sum;Note, I made two other changes:
- I fixed your indentation (watch that in the future)
- I turned your while loop into a for loop. An incrementation at the end of a while loop like you is a pretty good indication you need a for loop.
Now, instead of having three functions, you can just have this one function and call it with different
exp values.BUT WAIT!! You can make your program even more flexible now! Rather than having the user enter the "type" of exponent series thing they want, have them input these three things:
- A number
- The amount of times to add that number
- The exponent to raise the number to when adding
That way, you won't even need a switch statement at all! The user could enter numbers like
3, or 8, or anything else without having to be restricted by what options you already set in place!Code Snippets
std::cout << restraint << std::endl;
std::cout << request_range_1;
std::cin >> range_1;
std::cout << request_range_2;
std::cin >> range_2;if(choice == 'd')
break;
}
std::cout << "Calculator OFF" << std::endl;case 'd':
std::cout << "Calculator OFF" << std::endl;
return 0;int_sum += a;squ_sum += a*a;Context
StackExchange Code Review Q#119711, answer score: 2
Revisions (0)
No revisions yet.