patterncMinor
Summing Divisors
Viewed 0 times
summingdivisorsstackoverflow
Problem
Definition: A proper divisor of a natural number is the divisor that
is strictly less than the number.
e.g. number 20 has 5 proper divisors: 1, 2, 4, 5, 10, and the divisor
summation is: 1 + 2 + 4 + 5 + 10 = 22.
Input
An integer stating the number of test cases (equal to about 200000),
and that many lines follow, each containing one integer between 1 and
500000 inclusive.
Output
One integer each line: the divisor summation of the integer given
respectively.
Example
Sample Input:
Sample Output:
I submitted this problem on spoj and it is telling that time limit exceeded, how to decrease time?
is strictly less than the number.
e.g. number 20 has 5 proper divisors: 1, 2, 4, 5, 10, and the divisor
summation is: 1 + 2 + 4 + 5 + 10 = 22.
Input
An integer stating the number of test cases (equal to about 200000),
and that many lines follow, each containing one integer between 1 and
500000 inclusive.
Output
One integer each line: the divisor summation of the integer given
respectively.
Example
Sample Input:
3
2
10
20Sample Output:
1
8
22I submitted this problem on spoj and it is telling that time limit exceeded, how to decrease time?
#include
int main()
{
int num,divisor,sum,i,j,test_cases;
//printf("Enter the number of test_cases");
scanf("%d",&test_cases);
for(i=0;i<test_cases;i++)
{
sum=0;
// printf("Enter the numbers");
scanf("%d",&num);
for(j=1;j<num;j++)
{
divisor=num%j;
if(divisor==0)
{
sum=sum+j;
}
}
printf("%d\n",sum);
}
return 0;
}Solution
There are a few comments on this already, but I really want to point out a few things here.
Single responsibility
Loading all the functionality in to the main method is making your code hard to read, and reuse. You should isolate logic in to separate methods whenever you can. In this case, the user-input and the actual sum calculation should be in different places.
Variables
Your naming for values is poor. Each variable (apart from
In addition, variables should be declared where they are used. pre-declaring all variables at the top of a function is a requirement of old versions of C (from last century), and you should be doing better.
Precision
You have declared your variables as int. This could be a problem for systems where int is just 16 bits. You need to store the values up to 5000000, and this requires at least 32 bits. A
Style
You do not have enough white-space in your code. Also called breathing room, or suffocation:
What can't that be written as:
etc.
Performance
Your code loops for divisors from 1 to just less than
So, for example, the input 20. The square-root of 20 is 4.47... We know:
Our factors for 20 are thus 1, 2, 10, 4, 5, and 20 itself.
The rules for this challenge exclude the number itself, so 20 is not part of it.
The sum of factors is thus: 1 + 2 + 4 + 5 + 10 = 22
In order to find all factors, you only need to loop through to \$\sqrt{num}\$, and you need special handling for the input value 1, and when the root itself is an exact factor of the number.
Conclusion
Putting this all together, I propose the following:
Single responsibility
Loading all the functionality in to the main method is making your code hard to read, and reuse. You should isolate logic in to separate methods whenever you can. In this case, the user-input and the actual sum calculation should be in different places.
Variables
Your naming for values is poor. Each variable (apart from
sum) has a problem:int num,divisor,sum,i,j,test_cases;numshould benumber. Why abbreviate it?
divisoris misleading. When you do division, you take the dividend, divide it by the divisor, and the result is the quotient. The quotient has a whole part, and a remainder. What you call thedivisoris actually theremainder. What you calljis actually the divisor.
i- not a bad name for a loop variable (test cases), but I would go with something more meaningful, likeproblem, but, then again, you only use it as the control variable for a loop, and really you can replace the for-loop with a while-loop.
j- already covered this, it should bedivisor(anddivisorshould be something else.
test_cases- you are not running tests. You are solving problems. Call it what it is:problem_count.
In addition, variables should be declared where they are used. pre-declaring all variables at the top of a function is a requirement of old versions of C (from last century), and you should be doing better.
Precision
You have declared your variables as int. This could be a problem for systems where int is just 16 bits. You need to store the values up to 5000000, and this requires at least 32 bits. A
long is required, and keeping it unsigned would be fine.Style
You do not have enough white-space in your code. Also called breathing room, or suffocation:
int main() {
int num,divisor,sum,i,j,test_cases;
//printf("Enter the number of test_cases");
scanf("%d",&test_cases);
for(i=0;i<test_cases;i++)
{
sum=0;What can't that be written as:
int main()
{
int num, divisor, sum, i, j, test_cases;
//printf("Enter the number of test_cases");
scanf("%d", &test_cases);
for(i = 0; i < test_cases; i++)
{
sum = 0;etc.
Performance
Your code loops for divisors from 1 to just less than
num. There is no need to loop this far. Each time you find a factor for your number, you actually find 2, the low, and high parts of the factor. If you loop to the square root of the number, then that's all you need to do, except, for each factor less than the root, you also find one larger than the root. The exception is if the root of the number is also a factor.... then that only counts as one factor, not two.So, for example, the input 20. The square-root of 20 is 4.47... We know:
- 1 is the first factor of 20
- 2 is a factor (and 20 / 2 is 10, which is the high factor).
- 3 is not a factor
- 4 is a factor (and 20 / 4 is 5, which is the high factor).
- 5 is larger than 4.47... so we stop at there.
Our factors for 20 are thus 1, 2, 10, 4, 5, and 20 itself.
The rules for this challenge exclude the number itself, so 20 is not part of it.
The sum of factors is thus: 1 + 2 + 4 + 5 + 10 = 22
In order to find all factors, you only need to loop through to \$\sqrt{num}\$, and you need special handling for the input value 1, and when the root itself is an exact factor of the number.
Conclusion
Putting this all together, I propose the following:
#include
#include
unsigned long sumDivisors(unsigned long number)
{
if (number = 0)
{
unsigned long number = 0;
// printf("Enter the numbers");
scanf("%lu", &number);
unsigned long sum = sumDivisors(number);
//printf("%lu -> %lu\n", number, sum);
printf("%lu\n", sum);
}
return 0;
}Code Snippets
int num,divisor,sum,i,j,test_cases;int main() {
int num,divisor,sum,i,j,test_cases;
//printf("Enter the number of test_cases");
scanf("%d",&test_cases);
for(i=0;i<test_cases;i++)
{
sum=0;int main()
{
int num, divisor, sum, i, j, test_cases;
//printf("Enter the number of test_cases");
scanf("%d", &test_cases);
for(i = 0; i < test_cases; i++)
{
sum = 0;#include <stdio.h>
#include <math.h>
unsigned long sumDivisors(unsigned long number)
{
if (number <= 1)
{
return number;
}
// start sum at 1, and then loop from 2 later to exclude the number itself.
// use unsigned long
unsigned long sum = 1;
unsigned long root = (unsigned long)sqrt((double)number);
for(unsigned long divisor = 2; divisor <= root; divisor++)
{
int remainder = number % divisor;
if(remainder == 0)
{
unsigned long high = number / divisor;
sum += divisor;
if (high != divisor)
{
// only add high when the number is not a square number.
sum += high;
}
}
}
return sum;
}
int main()
{
unsigned long problem_count;
//printf("Enter the number of test_cases");
scanf("%ul", &problem_count);
while(--problem_count >= 0)
{
unsigned long number = 0;
// printf("Enter the numbers");
scanf("%lu", &number);
unsigned long sum = sumDivisors(number);
//printf("%lu -> %lu\n", number, sum);
printf("%lu\n", sum);
}
return 0;
}Context
StackExchange Code Review Q#73445, answer score: 7
Revisions (0)
No revisions yet.