HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Summation calculator of integers, squares, and cubes

Submitted by: @import:stackexchange-codereview··
0
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:

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.