Project Euler #1 - Multiples of 3 and 5 - First Program

Question

Taking a problem from the website, "Project Euler #1", I have created my first C# program. The problem I have used is as follows:

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.

The program I have made:

using System;

namespace _5_multiples
{
    public class MainClass
    {
        public static void Main()
        {
            int currentNumber = 1000;
            int allSum = 0;

            currentNumber = currentNumber - 1;
            while (currentNumber > 0)
            {
                if (currentNumber % 3 == 0)
                {
                    allSum = allSum + currentNumber;
                }
                else
                {
                    if (currentNumber % 5 == 0)
                    {
                        allSum = allSum + currentNumber;
                    }
                }
                currentNumber = currentNumber - 1;
            }
            Console.WriteLine(allSum);
        }
    }
}

I understand there is probably a lot to improve - this is my first C# program after all. I did run into a few problems such as referencing non static members from a static context and no suitable entry method. Rather than finding a definitive fix, the solution I implemented was more of a workaround; hence why I feel there may be a lot to improve.

namespace _5_multiples - this doesn't look right? Name of the program is actually 3_5_multiples - perhaps a badly chosen name?

Anyway, please let me know on anything that could be improved.

Answer 1

Here's what I would change:

• I'd use a for loop instead of a while loop here: for (int currentNumber = 999; currentNumber > 0; currentNumber--). This keeps all loop-related things grouped together. Loop bodies can be skipped or broken out of, so if code that must be executed every cycle is put at the end of a loop body it's easily broken when the loop body grows more complex and someone adds a continue or break in there.
• Actually, there is no need to count down - the result will be the same either way. Counting up is often a little easier to understand: for (int currentNumber = 0; currentNumber < 1000; currentNumber++). Note that using < here means you don't need to decrement currentNumber by one before the loop starts.
• You can write a += b instead of a = a + b (the same goes for various other operators).
• else { if { .. } } can be simplified to else if { ... }.
• Though in this case, a single if statement is sufficient: if (currentNumber % 3 == 0 || currentNumber % 5 == 0) { ... }. The 'or' operator (||) short-circuits if the first expression is true, so if a number can be divided by 3 it won't also try to divide it by 5.
• C# naming conventions typically write class, method and namespace names in PascalCase.

Regarding your questions:

• Static things belong to a class, non-static things belong to instances of that class. Classes are to instances what blueprints are to actual buildings. Trying to access a non-static (instance) members from a static context is like trying to open a door on a blueprint. The blueprint doesn't contain any real doors, only actual buildings do.
• Programs need a place to start executing, and in C# the starting point is a static Main method. Putting everything in there is fine for a small program like this.
• Identifier names in C# cannot start with a number, so 3_5_multiples isn't a valid name. I'd probably use EulerProblem1 or something like that here.
• Namespaces are useful for organizing larger programs (namespaces to classes are a bit like folders to files). The root namespace of a project is usually the name of that project itself, and child namespaces often correspond to folders.

EDIT: Part of the fun on Project Euler is coming up with more efficient solutions. Can you find a way to solve this without checking every number below 1000?

Answer 2

I have edited my program following the advice given on this thread:

using System;

namespace EulerProblemOne
{
    public class MainClass
    {
        public static void Main()
        {
            int allSum = 0;
            Console.WriteLine("Enter a number...");
            int inputNumber = Convert.ToInt32(Console.ReadLine());
            inputNumber -= 1;

            for (int currentNumber = inputNumber; currentNumber > 0; currentNumber--)
                if ((currentNumber % 3 == 0) || (currentNumber % 5 == 0))
                {
                    allSum += currentNumber;
                }
            Console.WriteLine(allSum); 
        }
    }
}

I now have the following:

• Renamed the program so that it no longer starts with a number.
• The number to check under can now be inputted by the user and not a fixed literal.
• A for loop is used instead of a while loop.
• IF statements have been combined into a single IF statement using OR.
• Arithmetic lines now use add/subtract and assign in a single operator.

Answer 3

You can just use the loop counter for currentNumber
Can use an or for divisible

public static int Numbers(int toNumber)
{
    int allSum = 0;
    for (int i = 3; i < toNumber; i++)
    {
        if (i % 3 == 0 | i % 5 == 0)
        {
            allSum += i;
        }
    }
    Debug.WriteLine(allSum);
    return allSum;
}

Answer 4

    using System;

namespace _5_multiples
{
    public class MainClass
    {
        public static void Main()
        {
            int currentNumber = 1000;
            int allSum = 0;

            currentNumber = currentNumber - 1;
            while (currentNumber > 0)
            {
                if ( (currentNumber % 3 == 0) | (currentNumber % 5 == 0))
                {
                    allSum = allSum + currentNumber;
                }
                currentNumber = currentNumber - 1;
            }
            Console.WriteLine(allSum);
        }
    }
}

This is my advice, you didn't need both -if-, only one is necessary.