Project Euler Wrong answer

[EwCjE3v7]

This code gives me the wrong answer after researching online for the answer. Can someone tell me why?

Problem 1:

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.

 #include <iostream>
#include <vector>
using std::cout; using std::endl; using std::cin;
using std::vector;

int main()
{
    int val = 3, counter = 1, num = 1, counter2 = 0;
    while (num <= 1000) {
        while (val * counter != num) {
            ++num;
        }
        if (val * counter == num) {
            counter2 += num;
            ++counter;
        }
    }
    val = 5, counter = 1, num = 1;
    
    while (num <= 1000) {
        while (val * counter != num) {
            ++num;
        }
        if (val * counter == num) {
            counter2 += num;
            ++counter;
        }
    }
    
    cout << counter2 << endl;
    
    return 0;
}

[JLBorges]

How many times are you adding 15 to counter2? Once or twice?
What about 30, 45 ... ?

[Nogroth]

You could simplify your code a good deal using the % operation (modular division) and the 'or' logical operator.
http://www.cplusplus.com/doc/tutorial/operators/
It would also fix your issue, which JLBorges already covered.

[JLBorges]

> You could simplify your code a good deal using the % operation (modular division)
> and the 'or' logical operator

How?

Sum of numbers 1,2,3,4, ....., n == ( n * (n+1) ) / 2

Sum of numbers 3,6,9,....,999 == 3 * ( sum of numbers 1,2,3,4,....,333)
                              == 3 * (333*334) / 2

Sum of numbers 5,10,15,....,995 == 5 * ( sum of numbers 1,2,3,4,....,199)
                                == 5 * (199*200) /2

Sum of numbers 15,30,45,...,990 == 15 * (66*67) / 2

Take it up from there.

[EwCjE3v7]

Jan 29, 2014 at 7:30pm
JLBorges (4075)
How many times are you adding 15 to counter2? Once or twice?
What about 30, 45 ... ?
Report

What do you mean

Jan 30, 2014 at 2:53am
LearningTheKingsOfTrades (23)
You could simplify your code a good deal using the % operation (modular division) and the 'or' logical operator.
http://www.cplusplus.com/doc/tutorial/operators/
It would also fix your issue, which JLBorges already covered.
Last edited on Jan 30, 2014 at 2:55am Report

I'll try that.

[JLBorges]

> What do you mean

This is what you are doing:

Q: Find the sum of all the multiples of 3 or 5 below 25

A: Sum up all multiples of 3 below 25: 3 + 6 + 9 + 12 + 15 + 18 + 21
   To that, add all multiples of 5 below 25: + 5 + 15 + 20 

15 is being added twice, once as a multiple of three, and then again as a multiple of five.

[kevinkjt2000]

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

The importance is the word or.

The modulus that was mentioned earlier returns the remainder of a division.
if((number % 3 == 0) || (number % 5 == 0))

[EwCjE3v7]

Oh oh oh

Thanks guys, I know what I`m doing wrong now. Thanks

[j3Rz8vqX]

Our tips would require you to change your original design; slightly, due to the use of two conditions on if - only one while loop.

A possible spoiler below:
Ah, no spoiler tag.
http://pastie.org/8683012

There's no one way about any problem.

[JLBorges]

Spoiler: http://coliru.stacked-crooked.com/a/dedcd6f02547f1d7

[EwCjE3v7]

Sorry the code your provided is wrong.

#include <iostream>
using namespace std;

int main (){
    int counter=0, num=0, first=3, second=5;
    while (num <= 10) {
        if (((num%first==0) || (num%second==0) )&& (num>=first)) {
            ++counter;
        }
        num++;
    }
    cout<<counter<<endl;
    return 0;
}

[j3Rz8vqX]

Of course it wouldn't simply answer your problem.
The spoiler was design to demonstrate JLBorges and kevinkjt2000's comment about OR and was designed to tip you in the right direction of not counting duplicate multiples of 3 and 5. X|

Since this is solved, another appropriate answer would be:

#include <iostream>
using namespace std;

int main (){
    int counter=0, num=0, first=3, second=5;
    while (num < 10) {
        if (((num%first==0) || (num%second==0) )&& (num>=first)) {
            cout<<num<<endl;
            counter+=num;
        }
        num++;
    }
    cout<<counter<<endl;
    return 0;
}

[Chervil]

Since the problem calls for "list all the natural numbers", that is 1, 2, 3, ....
then the initial value num=0 should be num=1 and that means the code can be simplified, there is no need to put && (num>=first).

[j3Rz8vqX]

natural numbers

You're correct Chervil.

Counting from 0 has become habitual.

Thank you for the correction.