Return to Answer

The thing that bothers me most about your code (and most of the answers) is that they are really long and verbose. I'd probably roll with something like the following:

public class Euler3
{
    public static int LargestPrimeFactorOf(int n)
    {
        return PrimeFactorsOf(n).Max();
    }

    private static IEnumerable<int> PrimeFactorsOf(int n)
    {
        for (var cand = 2; cand <= Math.Sqrt(n); cand++)
            for (; n % cand == 0; n /= cand)
                yield return cand;

        if (n > 1) yield return n;
    }
}

It has the "benefit" of being lazy in it's factorization method (which may or may not be a good thing, in this case it doesn't add much since we need to iterate over all factors to find the largest). I also find it to be more consice and declarative.

I also see no need whatsoever to make these instance methods since they are pure mathematical functions who depend only on their input. You spoke of OOP, but this is not a problem that is suited for OOP code. It's too small scale. We are designing an algorithm, we're not buildinga complex system.

The thing that bothers me most about your code (and most of the answers) is that they are really long and verbose. I'd probably roll with something like the following:

public class Euler3
{
    public static long LargestPrimeFactorOf(long n)
    {
        return PrimeFactorsOf(n).Max();
    }

    private static IEnumerable<long> PrimeFactorsOf(long n)
    {
        for (var cand = 2; cand <= Math.Sqrt(n); cand++)
            for (; n % cand == 0; n /= cand)
                yield return cand;

        if (n > 1) yield return n;
    }
}

It has the "benefit" of being lazy in it's factorization method (which may or may not be a good thing, in this case it doesn't add much since we need to iterate over all factors to find the largest). I also find it to be more consice and declarative.

I also see no need whatsoever to make these instance methods since they are pure mathematical functions who depend only on their input. You spoke of OOP, but this is not a problem that is suited for OOP code. It's too small scale. We are designing an algorithm, we're not buildinga complex system.

The thing that bothers me most about your code (and most of the answers) is that they are really long and verbose. I'd probably roll with something like the following:

public class Euler3
{
    public static int LargestPrimeFactorOf(int n)
    {
        return PrimeFactorsOf(n).Max();
    }

    private static IEnumerable<int> PrimeFactorsOf(int n)
    {
        for (var cand = 2; cand <= n; cand++)
            for (; n%cand == 0; n /= cand)
                yield return cand;
    }
}

It has the "benefit" of being lazy in it's factorization method (which may or may not be a good thing, in this case it doesn't add much since we need to iterate over all factors to find the largest). I also find it to be more consice and declarative.

I also see no need whatsoever to make these instance methods since they are pure mathematical functions who depend only on their input. You spoke of OOP, but this is not a problem that is suited for OOP code. It's too small scale. We are designing an algorithm, we're not buildinga complex system.

The thing that bothers me most about your code (and most of the answers) is that they are really long and verbose. I'd probably roll with something like the following:

public class Euler3
{
    public static int LargestPrimeFactorOf(int n)
    {
        return PrimeFactorsOf(n).Max();
    }

    private static IEnumerable<int> PrimeFactorsOf(int n)
    {
        for (var cand = 2; cand <= Math.Sqrt(n); cand++)
            for (; n % cand == 0; n /= cand)
                yield return cand;

        if (n > 1) yield return n;
    }
}

It has the "benefit" of being lazy in it's factorization method (which may or may not be a good thing, in this case it doesn't add much since we need to iterate over all factors to find the largest). I also find it to be more consice and declarative.

I also see no need whatsoever to make these instance methods since they are pure mathematical functions who depend only on their input. You spoke of OOP, but this is not a problem that is suited for OOP code. It's too small scale. We are designing an algorithm, we're not buildinga complex system.

The thing that bothers me most about your code (and most of the answers) is that they are really long and verbose. I'd probably roll with something like the following:

public class Euler3
{
    public static int LargestPrimeFactorOf(int n)
    {
        return PrimeFactorsOf(n).Max();
    }

    private static IEnumerable<int> PrimeFactorsOf(int n)
    {
        if (n < 2) yield break;
        for (var cand = 2; cand <= n; cand++)
            for (; n%cand == 0; n /= cand)
                yield return cand;
    }
}

It has the "benefit" of being lazy in it's factorization method (which may or may not be a good thing, in this case it doesn't add much since we need to iterate over all factors to find the largest). I also find it to be more consice and declarative.

I also see no need whatsoever to make these instance methods since they are pure mathematical functions who depend only on their input. You spoke of OOP, but this is not a problem that is suited for OOP code. It's too small scale. We are designing an algorithm, we're not buildinga complex system.

The thing that bothers me most about your code (and most of the answers) is that they are really long and verbose. I'd probably roll with something like the following:

public class Euler3
{
    public static int LargestPrimeFactorOf(int n)
    {
        return PrimeFactorsOf(n).Max();
    }

    private static IEnumerable<int> PrimeFactorsOf(int n)
    {
        for (var cand = 2; cand <= n; cand++)
            for (; n%cand == 0; n /= cand)
                yield return cand;
    }
}

It has the "benefit" of being lazy in it's factorization method (which may or may not be a good thing, in this case it doesn't add much since we need to iterate over all factors to find the largest). I also find it to be more consice and declarative.

I also see no need whatsoever to make these instance methods since they are pure mathematical functions who depend only on their input. You spoke of OOP, but this is not a problem that is suited for OOP code. It's too small scale. We are designing an algorithm, we're not buildinga complex system.