How is my code wrong

[Traitor4000]

how is my code wrong i believe the answer is supposed to be 233168 however it outputs 234168. Here is the problem http://projecteuler.net/problem=1  did the guy who said what it should come out to be on a forum have a typo and it really is the right anser


anyways here is what i wrote

Code: [Select]

#include <stdio.h>


int mults;
int multiples(void);

int main(void)
{
mults = multiples();
printf("%d", mults);
return 0;
}

int multiples(void)
{
int cnt, mult3=0, mult5=0, answer;
   for(cnt=0;cnt<=1000;cnt++)
   {
      if(cnt%3 == 0)
      {
      mult3 += cnt;
      }
      else if(cnt%5 == 0)
      {
      mult5 += cnt;
      }
     }   
answer = mult5 + mult3;
return answer;
}


[Code.Illusionist]

Because you forgot numbers as 15, 30... 15 can be devided either by 3, and by 5 and give 0 to remain. So when your if statement come to it, what will it do?
Code:

Code: [Select]

#include <stdio.h>
int multiples(void);

int main(void)
{
int mults = 0;   
mults = multiples();
printf("%d", mults);
return 0;
}

int multiples(void)
{
int cnt, mult = 0, answer = 0;
   for(cnt=0;cnt<1000;cnt++)
   {
      if( cnt%3 == 0 || cnt%5 == 0)
      {
      mult += cnt;
      }
      else if( cnt % 3 == 0 && cnt % 5 == 0)
      {
      mult += cnt;
      }
 }
answer = mult;
return answer;
}Now, what you did wrong is you set loop to be done until it's either less or equal to 1000. And you don't need that. What you need is only loop to be done until number is less then 1000. Why? Because your application start counting from 0 till 999 which is exactly 1000 times. When the code is compiled you will get number you wanted.

http://ideone.com/uTxOG5

[I_Learning_I]

Your if's are correct, Code.Illusionist didn't see the else I suppose.
But he is right, you're if is doing 1 too many times, should be <1000.

Code: [Select]

http://ideone.com/S8C7aj
Just to keep some indentation the way I like it (It's also the way it's written in ISO C book), and removing that || followed by an  &&(no point for them, since there's an else, would never happen.


Rules of OR:
True + False = True
False + True = True
True + True=True
False + False = False.



Rules of AND:
True + False = False
False + True = False
True + True=True
False + False = False.






So you shoudl neve do this:

Code: [Select]

if(a||b) //As soon as this becomes true it will skip the next IF, if it's false the next will be false too and it will skip it
else if (a && b)


[ArkPhaze]

If you're going to solve the harder questions on Project Euler you need to start thinking about the limitations of the questions and in this case the numbers you are generating. Regardless, what you did with the code is the way probably 80% of any person trying to solve that question, would have done, keep in mind.

Here's something I've put together for Euler #1 in C:

Code: [Select]

int main(void)
{
   int sum = 0;
   const int limit = 1000;
   for (int i = 3, j = 5; i < limit; i += 3)
   {
      sum += i;
      while (j < i)
      {
         if (j % 3) sum += j;
         j += 5;
      }
   }
   printf("%d\n", sum);
}
I'm using C99 btw. This is the way you should be thinking about Project Euler questions though, as they are going to get MUCH more difficult. Thinking differently about how you code, also makes you a better programmer.

What you are doing is checking all values in that range, why though? You know that out of every 3 numbers, only 1 of them is going to be divisible by 3, and the same logic applies with multiples of 5; every 5th number. Why not just count the multiples and filter out the numbers that are both multiples of 3 and 5 to avoid having added it twice to the sum? :S

Examples of those numbers are obviously, 15, 30, etc...

Also, checking and adding 0, in any case, would be a waste of time, because a number added to 0, is the same as the number it previously was. In your case I would've at least started with 3. Checking 0, 1, 2, 4, etc... Is just a waste of CPU time.

In your case though, I don't see the reasoning behind having a variable outside of a local scope like you have mults currently. If you add the values of mult3 and mult5, why have separate variables for each if you don't use them as individual values?

Project Euler is a great way to apply your knowledge though and progress in a language you are trying to learn.

[Traitor4000]

Thanks for all the responses now I feel kind of stupid should have been able to figure that out myself. @ArkPhase Yeah I'll try to think more outside the box for more efficient solution oh yeah and I use C89 just for portability not that portability really matters for Euler    but I try to keep good habits.