the sum of divisor (other than itself)

Question

Write a program that reads integer from the keyboard and, on the output, writes the sum of the divisors of n (other than itself).

I've created a method that finds the sum of divisor. Using the while statement I can have up to 10 integers entered by the user until EOF. In the while statement I had sum = the output of sum_divisor and print out sum.

If I enter 0 0 0 4 5 6 12 then the output should be 0 0 0 3 1 6 16. The output I get is 0 0 0 3 4 10 26. How can I make it so that it doesn't add the sum_divisor to the previous sum?

#include <stdio.h>

int sum_divisor(int x);

int main()
{
    int x;
    int sum;
    printf("Enter up to 10 positive integer ending with EOF:\n");

    while((scanf("%d",&x)) != EOF){
        sum = sum_divisor(x);
        printf("%d ", sum);
    }
    return 0;
}

int sum_divisor(int x){

    int i;
    int sum;

    if(x<= 0){
        sum = 0;
    }
    else{
        for(i=1;i<x;++i)
            {
                if(x%i==0)
                    sum += i;
            }
    }
    return sum;
}

Answer 1

You should initialise sum to zero within your function. Otherwise, it will be set to some arbitrary value and the else block will have an arbitrary result.

In other words, change:

int sum;

into:

int sum = 0;

Of course, once you've done that, there's no need to explicitly do anything for the case where x is less than one. In addition, the initial if is superfluous since the for body won't execute when x is less than one, so you could get away with something like:

int sumDivisors (int x) {
    int i, sum = 0;

    for (i = 1 ; i < x; i++) {
        if ((x % i) == 0) {
            sum += i;
        }
    }
    return sum;
}

---

As an aside, the values you're actually seeing without the initialisation are accumulating:

 0 ->  0
 0 ->  0
 0 ->  0
 3 ->  3
 1 ->  4
 6 -> 10
16 -> 26

This is almost certainly because each call to the function is reusing the same memory for the stack frame, including the variable sum, so sum is simply being added to each time (that the passed-in parameter is greater than one).

However, that's simply an artifact of the implementation, it's not guaranteed by the standard, which states quite clearly in C11 6.7.9 Initialization /10:

If an object that has automatic storage duration is not initialized explicitly, its value is indeterminate.

In other words, don't rely on this.

---

And, as yet another aside, it's a mathematical given that, if a number n divides evenly by a, it also divides evenly by n/a. You can use this to your advantage to possibly make the code a little more efficient (though, as with all optimisations, you should measure, not guess).

Since you're discounting the number itself as a divisor, you have to treat the divisor 1 as a special case. You also have to treat perfect squares as a special case so that you don't add the square root twice.

The following code would be a good starting point for that:

int sumDivisors (int x) {
    int i, sum;

    // Always return zero for -inf..1 inclusive.

    if (x < 2)
        return 0;

    // Otherwise, 1 is factor, search for others
    //   up to but NOT including sqrt(x).
    for (i = 2, sum = 1 ; i * i < x; i++) {
        if ((x % i) == 0) {
            sum += i;
            sum += x / i;
        }
    }

    // Add in sqrt(x) ONCE for a perfect square.

    if (i * i == x)
        sum += i;

    return sum;
}

Answer 2

When you call sum_divisor inside the while statement the same block of stack is being allocated, and therefore the sum variable is allocated in the same position on every call. At the first call the value is 0,(but not necessarily), the next time the value will be the one you calculated on the previous call.

int sum = 0; should fix it.

Answer 3

int sum_divisor(int x){
    if(x <= 0) { // you do not need to even alocate space for i an sum if x < 1.
      return 0;
    }

    int i = 1;
    int sum = 0;

    for(i=1;i<x;++i)
    {
        if(x%i==0)
            sum += i;

    }
    return sum;
}