HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Project Euler Attack #2

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
attackprojecteuler

Problem

This is my implementation of Project Euler #2 in C#. It takes 0 milliseconds to run, and I believe it is correct - I have seen nothing to indicate otherwise!


By considering the terms in the Fibonacci sequence whose values do not exceed four million, find the sum of the even-valued terms.

static void GetFibos(ref List nums, int maxVal)
{
    nums.Add(1);
    int lastNum = 2;

    while (lastNum  fiboNums = new List();
    GetFibos(ref fiboNums, 4000000);

    int sumEvenFibos = 0;

    foreach (int i in fiboNums) { if (i % 2 == 0) sumEvenFibos += i; }

    s.Stop();

    Console.WriteLine(sumEvenFibos);
    Console.WriteLine(s.ElapsedMilliseconds);
}


Is there anything I should be doing different?

Solution

Why do you need the actual list of Fibonacci numbers? That just requires space, and time.

Believe it or not, you have over-thought this problem. Additionally, your code style is off.

C# Code conventions always use braces for single-statement blocks, and the brace should open at the start of a new line. Code like:

foreach (int i in fiboNums) { if (i % 2 == 0) sumEvenFibos += i; }


should be:

foreach (int i in fiboNums)
{
    if (i % 2 == 0)
    {
        sumEvenFibos += i;
    }
}


As for the list, you don't need it:

int current = 1;
int previous = 0;
int sum = 0;
while (current <= 4000000)
{
    if ((current % 2) == 0)
    {
        sum += current;
    }
    int tmp = current + previous;
    previous = current;
    current = tmp;
}


Now, that whole thing should be put in its own function, and your main method with a parameter for the upper limit (4,000,000) and it becomes:

static void Main(string[] args)
{
    Stopwatch s = new Stopwatch();
    s.Start();

    int sumEvenFibos = GetFiboSum(4000000);

    s.Stop();

    Console.WriteLine(sumEvenFibos);
    Console.WriteLine(s.ElapsedMilliseconds);
}

Code Snippets

foreach (int i in fiboNums) { if (i % 2 == 0) sumEvenFibos += i; }
foreach (int i in fiboNums)
{
    if (i % 2 == 0)
    {
        sumEvenFibos += i;
    }
}
int current = 1;
int previous = 0;
int sum = 0;
while (current <= 4000000)
{
    if ((current % 2) == 0)
    {
        sum += current;
    }
    int tmp = current + previous;
    previous = current;
    current = tmp;
}
static void Main(string[] args)
{
    Stopwatch s = new Stopwatch();
    s.Start();

    int sumEvenFibos = GetFiboSum(4000000);

    s.Stop();

    Console.WriteLine(sumEvenFibos);
    Console.WriteLine(s.ElapsedMilliseconds);
}

Context

StackExchange Code Review Q#77181, answer score: 6

Revisions (0)

No revisions yet.