3
\$\begingroup\$

If we list all the natural numbers below 10 that are multiples of 3 or 5, we get 3, 5, 6 and 9. The sum of these multiples is 23.

Find the sum of all the multiples of 3 or 5 below 1000.

Can someone give me suggestions on how to improve this solution? Also, please tell me whether or not my programming style is good.

#include<stdio.h>

int main()
{
    int a,b,c,d,sum3,sum5,sum15,total_sum;
    b=999/3;
    c=999/5;
    d=999/15;
    sum3=b*(3+999)/2;
    sum5=c*(5+995)/2;
    sum15=d*(15+990)/2;
    total_sum=sum3+sum5-sum15;
    printf("%d",total_sum);
    return 0;
}
\$\endgroup\$
1
4
\$\begingroup\$

I would have implemented it like this:

printf("233168");

And solved it on paper. But I guess that's cheating :-)

I would probably write your code as:

#include <stdio.h>

int calculateSumOfMultiples()
{
    int sum  = ((999/3)  * (999 +  3))
             + ((999/5)  * (995 +  5))
             - ((999/15) * (990 + 15));

    return sum / 2;
}

int main()
{
    printf("%d\n", calculateSumOfMultiples());
    return 0;
}

The key different points are:

  • I think it's more readable to put the math into one equation like this. Using variables for temporary results is okay too, but then at least give the variables more meaningful names.

  • I perform the divison only once. It's mathematically equivalent, and gives less clutter.

  • I separate calculation from IO. I have a function that does all the work, and then call that function.

  • I use whitespace and aligning to make the code more readable.

To make the code even better, calculateSumOfMultiples could be a generic solution and take the number (1000) and multiples (3 and 5) as input. This is left as an exercise for the reader.

\$\endgroup\$
2
  • 3
    \$\begingroup\$ +1 for your point about printf("233168"). Interestingly, an optimizing compiler will perform that calculation at compile time, such that calculateSumOfMultiples() will not even get called — it just compiles to 233168. \$\endgroup\$ Oct 19 '13 at 7:40
  • \$\begingroup\$ If you're going to be stingy and postpone the division by 2, then rename sum to doubleSum. \$\endgroup\$ Oct 19 '13 at 7:42
4
\$\begingroup\$
  • Functions in C with no parameters should have a void parameter:

    int main(void)
    
  • Variables should be declared/initialized on separate lines:

    int a;
    int b;
    int c;
    int d;
    int sum3;
    int sum5;
    int sum15;
    int total_sum;
    
  • Try to avoid single-character variable names. They're very ambiguous and say nothing about the variable's purpose. The only valid exception to this is loop counter variables.

  • Statements/blocks of different purposes should be separated to increase readability. That would be the divisions, the sums, and the printf().

  • Your operands should be spread out to make the lines more readable:

    sum3 = b * (3+999) / 2;
    
  • Initializations should be done as late as possible to help aid in maintenance:

    sum3 = b * (3+999) / 2;
    
  • Remove int a since it's never used.

\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.