"for loop" error in c++

Question

I am trying to write a for loop to add up the sum between 2 integers 2-9=2+3+4+5...+9 and I used if statement to determine the larger integer in the input. Somehow, the for will execute itself once even though if condition return a false value. Therefore I keep getting the wrong value Here is the code I wrote. Thanks in advance.

#include <iostream>
using namespace std;
int main(){
    int in1, in2;
    void accumulate_sum(int i1, int i2);
    cout<<"Enter two integer:"<<endl;
    cout<<"Integer 1:"<<endl;
    cin>>in1;
    cout<<"Integer 2:"<<endl;
    cin>>in2;
    accumulate_sum(in1, in2);
    return 0;
}

void accumulate_sum(int i1, int i2){
    int i, sum, ca;
    if (i1 > i2)
    {
        ca=1;
        for (i=i2;i<(i1+1); i++) {
            sum+=i;
        }
    }
    else {
        i=i1;
        for (i;i<(i2+1); i++) {
            sum+=i;

        }
    }
    switch (ca) {
        case 1:
            cout<<"the sume from "<<i2<<" to "<<i1<<" is "<<sum<<endl;
            break;

        default:     cout<<"the sume from "<<i1<<" to "<<i2<<" is "<<sum<<endl;
            break;
    }
}

Answer 1

There's actually a mathematical formula to sum all the numbers from n to m, like this:

formula http://kepfeltoltes.hu/140916/latex_63cded397d9f50846f8c3c9e55e4af01_www.kepfeltoltes.hu_.png

Applying this formula makes your code much faster and cleaner, like so:

template <class T>
T accumulate_sum(T n, T m) {
    if (m<n) std::swap(n, m);
    return (m*(m+1))/2 - (n*(n+1))/2 + n;
}

Of course the template is not necessary, but if you call the function with an unsigned integer type, then casting it to int (which is signed) inside the function is not a good idea.

Be type safe (don't do unsigned-signed (or vice versa) conversions if possible), avoid unnecessary raw loops, and always use STL if you can! I'm not a C++ expert (and neither trying to pretend like I am), but it's good to keep these in mind!

Cheers

Answer 2

A simple solution:

The problem in your program exists because you aren't initializing sum to before you start adding values. You also check against ca in your switch even though there are paths where it's left uninitialized. Set them both to 0 where you declare them and everything should work.

---

Further Improvements/Things to ponder:

To improve, storing the min and max values can make your function a lot prettier and faster:

void accumulate_sum(int i1, int i2) {
    int sum = 0;
    int min = i1;
    int max = i2;
    if (i1 > i2) {
        min = i2;
        max = i1;
    }

    for (int i = min; i <= max; i++)
        sum += i;

    cout << "the sum from " << min << " to " << max " is " << sum << endl;
}

Using the science of partial sums, you can rewrite your for with a single formula. Since the sum of all natural numbers between 1 and n can be simplified as...

and we can prove that the sum of all natural numbers from min to max is equivalent to ( ( (sum from 1 to max) - (the sum from 1 to min) ) + min)...

then we can replace our for loop with the formula...

void accumulate_sum(int i1, int i2) {
    int min = i1;
    int max = i2;
    if (i1 > i2) {
        min = i2;
        max = i1;
    }

    int sum = (max*(max+1))/2 - (min*(min+1))/2 + min;
    cout << "the sum from " << min << " to " << max " is " << sum << endl;
}

Also, it's good practice to declare your functions outside of main() unless you have a really good reason for doing so:

// ...
using namespace std;
void accumulate_sum(int i1, int i2);
void main() {
// ...

Answer 3

try this

accumulate_sum(int integer1, int integer2){
     int min = integer1 < integer2 ? integer1 : integer2;
     int max = integer1 > integer2 ? integer1 : integer2;
     int sum = 0;
     for (int i = min; i < max; ++i){
          sum += i;
     }

     cout << "the sum of integers from " << min << " to " << max << " is " << sum;
}

you then eliminate the if and else statements and only have to write code once.