views:

1269

answers:

7

Hi there, I'm having some trouble with this problem in Project Euler.

Here's what the question asks:

Each new term in the Fibonacci sequence is generated by adding the previous two terms. By starting with 1 and 2, the first 10 terms will be: 1, 2, 3, 5, 8, 13, 21, 34, 55, 89, ... Find the sum of all the even-valued terms in the sequence which do not exceed four million.

My code so far: EDITED WITH NEW CODE THAT STILL DOESN'T WORK.

static void Main(string[] args)
{
    int a = 1;
    int b = 2;
    int Container = 0;
    int Sum = 0;

    while (b < 4000000)
    {
        if (a % 2 == 0)
        {
            Container += a;
        }

        Sum = a + b;
        a = b;
        b = Sum;
    }

    Container += b;

    Console.WriteLine(Container.ToString());
    Console.ReadLine();
}
+1  A: 

You're checking both a and b on every iteration. So that means you're double counting almost everything.

Edit:

Ok, I see your update. This is pretty basic debugging, and you should really learn to try it yourself. Think about what the values of a and b are when your loop condition stops being true.

recursive
Thanks for the feedback. I removed the checking of the B variable. So I only check if A is an even number. I'm still getting a wrong answer though. I don't know what could be wrong.
Sergio Tapia
I support @recursive's edit, and I'd add that you should check the values of `Container` throughout the loop.
Hosam Aly
A: 

It looks like you're adding each even Fibonacci number twice...

sonomax
+1  A: 

I think the question is written to say that you would add all the even numbers together while the numbers in the sequence don't exceed four million, meaning you would add 3,999,992.

somacore
"meaning you would add 3,999,992." Can you expand on this? Add 3,999,992 what?
Kirk Broadhurst
+6  A: 

One of the fun feature in C# is the "yield" keyword, which is very useful for this kind of thing:

IEnumerable<int> Fibonacci()
{
   int n1 = 0;
   int n2 = 1;

   yield return 1;
   while (true)
   {
      int n = n1 + n2;
      n1 = n2;
      n2 = n;
      yield return n;
   }
}

long result=0;

foreach (int i in Fibonacci().TakeWhile(i => i<4000000).Where(i % 2 == 0))
{
    result+=i;
}
Console.WriteLine(result);
Joel Coehoorn
@Joel - Great answer. I think you should change int i in Fibonacci().TakeWhile(i => i<4000000).Where(i % 2 == 0) to int i in Fibonacci().TakeWhile(i => i<4000000).Where(i=> i % 2 == 0)
sc_ray
+1  A: 

Joel, I wrote a very some similiar code; I'm posting it anyways:

static IEnumerable<int> Fibonacci(int maximum)
{
    int auxiliar = 0;
    int previous = 0;
    int current = 1;
    while (current < maximum)
    {
        auxiliar = previous;
        previous = current;
        current = auxiliar + current;
        yield return current;
    }
}

Console.WriteLine(Fibonacci(4000000).Where(number => number % 2 == 0).Sum());
Rubens Farias
+2  A: 
int sum = 2;
for(int f1 = 1, f2 = 2, f3 = 0; !((f3 = (f1 + f2)) > 4000000); f1 = f2, f2 = f3)
 sum += f3 * (~f3 & 1);
csharptest.net
+3  A: 

Your problem is not that your code contains a bug; your problem is that your code contains a bug and you don't know how to find it. Solve the second problem first, and then you won't need to ask us when you have a bug, you'll be able to find it yourself.

Learning how to find bugs is hard and takes a lot of practice. Here's how I would approach this problem.

I'd start by simplifying the problem down to something I could do myself. Instead of "what's the sum of the even fib numbers that do not exceed four million?" I'd ask "what's the sum of the even fib numbers that do not exceed 40?" That's easy to work out by hand -- 2 + 8 + 34 = 44.

Now run your program in a debugger, stepping through each line, and see where things go wrong. Does your program actually add up 2, 8 and 34? And if so, does it get the correct result?

Eric Lippert