Optimizing my solution for problem #23 - project Euler

Question

The question -

A perfect number is a number for which the sum of its proper divisors is exactly equal to the number. For example, the sum of the proper divisors of 28 would be 1 + 2 + 4 + 7 + 14 = 28, which means that 28 is a perfect number.

A number n is called deficient if the sum of its proper divisors is less than n and it is called abundant if this sum exceeds n.

As 12 is the smallest abundant number, 1 + 2 + 3 + 4 + 6 = 16, the smallest number that can be written as the sum of two abundant numbers is 24. By mathematical analysis, it can be shown that all integers greater than 28123 can be written as the sum of two abundant numbers. However, this upper limit cannot be reduced any further by analysis even though it is known that the greatest number that cannot be expressed as the sum of two abundant numbers is less than this limit.

Find the sum of all the positive integers which cannot be written as the sum of two abundant numbers.

My solution -

public class Problem23 {

    /**
     * A brute force solution - 
     * 1. Calculate the abundant numbers till the limit. 
     * 2. Check each number if it can be written as a sum of any two
     *    abundant numbers in the pre-calculated set
     * 
     * @param args
     */
    public static void main(String[] args) {

        Set<Integer> abundantNumSet = new LinkedHashSet<Integer>(); //sorted set of abundant numbers

        // find all abundant numbers from 13 to 28123, since 12 is the smallest
        // abundant number and greatest number that cannot be expressed as the
        // sum of two abundant numbers is less than 28123
        abundantNumSet.add(12);
        for (int i = 13; i < 28124; ++i) {
            if (isAbundant(i)) {
                abundantNumSet.add(i);
            }
        }

        int total = 276; // sigma 23, since 24 = 12 * 2
        for (int i = 25; i < 28124; ++i) {
            boolean isRep = false;

            for (Integer num : abundantNumSet) {
                // iterating through half of the numbers in the set is enough
                if (abundantNumSet.contains(i - num)) {
                    // the number can be represented as a sum of two
                    isRep = true;
                    break;
                }
                if (i < num) {
                    // the abundant number should less than a given number to
                    // be part of the numbers forming the sum
                    break;
                }
            }

            if (!isRep) {
                // the number cannot be represented as sum of two abundant
                // numbers
                total += i;
            }
        }

        System.out.println(total);
    }

    public static boolean isAbundant(int n) {
        int sum = 1;
        int end = (int) Math.sqrt(n);

        for (int i = 2; i <= end; ++i) {
            if (n % i == 0) {
                int quotient = n / i;

                if (quotient != i) {
                    sum += i + quotient; // add the factors
                } else {
                    // avoid adding twice - factor in case of perfect squares
                    sum += quotient;
                }
            }
        }

        if (sum > n) {
            return true;
        }

        return false;
    }
}

I would like to optimize my solution. Some pointers at the same would be appreciated.

Answer 1

I would like to optimize my solution.

Really? It's already pretty fast and there isn't much to gain, given that Java has some start up overhead. I'd save my power for later Eulers.

Set<Integer> abundantNumSet = new LinkedHashSet<Integer>();

Do you need "Linked"? I don't think so, so we've a tiny optimization.

    abundantNumSet.add(12);
    for (int i = 13; i < 28124; ++i) {
        if (isAbundant(i)) {
            abundantNumSet.add(i);
        }
    }

I really wouldn't do it. You're testing nearly 30k numbers, so is it worth to optimize the first 13? Moreover, the saved tests are the fastest.

You should use no magic constants like 12, 13, 28124. Define constants and give them proper names. This will help you to avoid unworthy optimizations. :D

    int total = 276; // sigma 23, since 24 = 12 * 2
    for (int i = 25; i < 28124; ++i) {

Another unworthy optimization, more magic numbers. What's worse, a repeated magic number. One day, in a big project you save a second by copying some numbers instead of defining constants. Another day, you change them all (it's easy, isn't it?). Yet another day, you'll be hunting a bug caused by having changed a completely unrelated number which by accident had the same value.

        boolean isRep = false;

The loop body is big enough for an own method. Then you can drop the variable, instead of spending time on naming it properly (isRep is not properly).

            // iterating through half of the numbers in the set is enough

This is a promise, you didn't keep. No problem, as this part takes much less time than the other, so speeding it twice helps nothing.

I'd go for a method like this (untested):

private boolean isSumOfTwoAbundants(int n) {
    for (int i : abundantNumSet) {
         if (i > n - i) {
             break;
         }
         if (abundantNumSet.contains(n - i)) {
             return true;
         }
    }
    return false;
}

When using things like this

    int end = (int) Math.sqrt(n);

be aware of how it get rounded. I always prefer Guava's

   IntMath.sqrt(int, RoundingMode)

as it's explicit about it.

    if (sum > n) {
        return true;
    }

    return false;

Do you really mean return sum > n?

I guess, isAbundant could be sped up by factorization:

• Similarly to the Sieve of Eratosthenes, you can precompute one factor for all numbers up to 28124.
• For every number n, you get one factor, which you can divide out and get another number and another factor etc., till you have the factorization of n.
• Having all prime factors (with their exponents), you can trivially compute the number of divisors.

This would probably help a lot, however, your code is already more then fast enough. While I love optimizing, I did about nothing for this problem and it took one second only (yours is about 10 times faster, do I care?).

Summary

You can optimize, but now it's time to learn not to optimize. Otherwise you can get quickly lost in the root of all evil.