3

I'm trying to solve problem 3 from http://projecteuler.net. However, when I run thing program nothing prints out. What am I doing wrong? Problem: What is the largest prime factor of the number 600851475143 ?

public class project_3 
{
    public boolean prime(long x)   // if x is prime return true
    {
        boolean bool = false;

        for(long count=1L; count<x; count++)
        {
            if( x%count==0 )
            {
                bool = false;
                break;
            }
            else { bool = true; }
        }
        return bool;
    }

    public static void main(String[] args)
    {
        long ultprime = 0L;  // largest prime value
        project_3 object = new project_3();

        for(long x=1L; x <= 600851475143L; x++)
        {
            if( object.prime(x)==true )
            {
                ultprime = ((x>ultprime) ? x : ultprime);
            }
        }
        System.out.println(ultprime);
    }
}
Will Ness
  • 62,652
  • 8
  • 86
  • 167
Dan
  • 55
  • 1
  • 2
  • 6
    `for(long x=1L; x<=600851475143L;x++)` - um... that's gonna take a while... – Mysticial Mar 07 '13 at 18:50
  • 2
    Start with `count=2L` – jonhopkins Mar 07 '13 at 18:50
  • 3
    And you should stop the `for-loop` on `sqrt(number)`. – Luiggi Mendoza Mar 07 '13 at 18:51
  • 1
    @BeauGrantham - Actually, it's fairly efficient; just incorrect. In the method `prime()`, since `count` starts at 1, `x%count` will be 0 on the first pass through the loop and every number will test as composite! – Ted Hopp Mar 07 '13 at 18:53
  • 7
    If you are looking for the largest then I would start in the opposite direction – Jason Sperske Mar 07 '13 at 18:53
  • 3
    @JasonSperske no you don't. To count down from 600851475143 is terribly inefficient. – Will Ness Mar 08 '13 at 11:34
  • Does nothing print out **ever** or are you just not waiting for the program to finish? – thecoshman Mar 08 '13 at 11:58
  • @WillNess: Why? If you're counting down, you're testing the same numbers that you would have had to test on the way up -- except you can drop out of the loop as soon as you get a hit. – BlairHippo Mar 08 '13 at 14:26
  • 1
    @BlairHippo because you'll have much *much* longer way to go. See my answer for details. It is very fast, and its speed is predicated on the counting up, not down. – Will Ness Mar 08 '13 at 14:36
  • Please check this link. http://stackoverflow.com/questions/18667400/printing-out-prime-numbers-from-2-to-1000/18667993?noredirect=1#comment27608316_18667993 – Braj Kishore Sep 13 '13 at 06:59

3 Answers3

6

Not only does your prime checking function always return false; even if it were functioning properly, your main loop does not seek the input number's factors at all, but rather just the largest prime smaller or equal to it. In pseudocode, your code is equivalent to:

foo(n):
    x := 0 ;
    foreach d from 1 to n step 1:
        if is_prime(d):          // always false
            x := d
    return x                     // always 0

is_prime(d):
    not( d % 1 == 0 )            // always false

But you don't need the prime checking function here at all. The following finds all factors of a number, by trial division:

factors(n):
    fs := []
    d  := 2
    while ( d <= n/d ):
        if ( n % d == 0 ): { n := n/d ; fs := append(fs,d) }
        else:              { d := d+1 }
    if ( n > 1 ): { fs := append(fs, n) }
    return fs

The testing for divisibility is done only up to the square root of the number. Each factor, as it is found, is divided out of the number being factorized, thus further reducing the run time. Factorization of the number in question runs instantly, taking just 1473 iterations.

By construction all the factors thus found are guaranteed to be prime (that's why no prime checking is needed). It is crucial to enumerate the possible divisors in ascending order for this to happen1. Ascending order is also the most efficient, because any given number is more likely to have smaller prime factor than larger one. Enumerating the primes instead of odds, though not necessary, will be more efficient if you have an efficient way of getting those primes, to test divide by.

It is trivial to augment the above to find the largest factor: just implement append as

append(fs,d):
    return d

1 because then for any composite divisor d of the original number being factorized, when we'll reach d, we will have already divided its prime factors out of the original number, and so the reduced number will have no common prime factors with it, i.e. d won't divide the reduced number even though it divides the original.

Will Ness
  • 62,652
  • 8
  • 86
  • 167
  • 1
    what's with the unexplained downvoting on a correct answer? I don't see one thing that's wrong in this answer. – Will Ness Feb 19 '16 at 09:50
  • 1
    Yours is the best answer by a long chalk but where you're wrong is that you apparently expect people to vote rationally and informedly. Just look at which of the comments on the question itself got the most votes... (no downvote possible for comments). The sheer numbers guarantee that myth, supposition and superstition (see Sieve of Atkin) are spreading unstoppably, despite the detailed, accurate debunkings that have been posted (as you know only too well). There are just too many of the half-clued who are spreading stuff they heard but didn't fully understand, without any due diligence. – DarthGizka May 09 '16 at 16:22
  • 1
    @DarthGizka it is what it is, I guess. :) both WP and SO subscribe to "one man one (possibly uninformed) vote" system (with nuances of course); consequently both are *not* about creating curated *knowledge* repositories, as much as we like to pretend they are... I've posted about 20 redundant answers just like this one, I guess (at least?); ideally there should have been only one -- if at all, as others did post them too. – Will Ness May 09 '16 at 21:31
3

Two things:

1) You are starting count at 1 instead of 2. All integers are divisible by 1.

2) You are running an O(n^2) algorithm against a rather large N (or at least you will be once you fix point #1). The runtime will be quite long.

Aurand
  • 5,214
  • 22
  • 35
  • Scroll down the code to see "System.out.println(ultprime);". But your other points are valid. – rgettman Mar 07 '13 at 18:54
  • Actually, the code _as written_ is not O(n^2) because the loop inside `prime()` runs exactly once (since `count` starts at 1). – Ted Hopp Mar 07 '13 at 18:55
  • 2
    Correction, if the algorithm wasn't broken it would be O(n^2) – Aurand Mar 07 '13 at 19:01
  • Consider using Sieve of Eratosthenes to find prime numbers http://en.wikipedia.org/wiki/Sieve_of_Eratosthenes – Steve Kuo Mar 07 '13 at 20:38
  • 3
    @SteveKuo That's good to find prime numbers, but it's not a good way to solve this kind of problem. To find the largest prime factor of a number, you have to factorise it (well, okay, you don't _have_ to, there are other ways). Simple factorisation by trial division is plenty good enough for a number of the given size, even if it is itself prime. – Daniel Fischer Mar 08 '13 at 13:36
  • 1
    the OP runs "an O(n^2) algorithm" which (after fixing the 1st pt) does *not* calculate the largest *prime factor* of a given number - just the largest *prime* not exceeding that number. – Will Ness Mar 09 '13 at 11:33
  • I estimate "quite long" as about a billion years (very very roughly). – starblue Mar 10 '13 at 15:46
2

The whole point of Project Euler is that the most obvious approaches to finding the answer will take so long to compute that they aren't worth running. That way you learn to look for the less obvious, more efficient approaches.

Your approach is technically correct in terms of whether or not it is capable of computing the largest prime of some number. The reason you aren't seeing anything print out is that your algorithm is not capable of solving the problem quickly.

The way you've designed this, it'll take somewhere around 4,000,000 years to finish.

If you replaced the 600851475143 number with say 20 it would be able to finish fairly quickly. But you have the 600 billion number, so it's not that simple.

user50511
  • 29
  • 2
  • 1
    Your estimate is about 4 million years off - simple trial division by odd numbers factors 600851475143 in about 25 microseconds in (C#!) because that number has only small factors. Even the product of the two primes that are just greater than its square root (775147 * 775153 = 600857522491) - which is the worst case for factoring by trial division - takes only 4 milliseconds. As always, Euler is several orders of magnitude away from being a challenge of any kind. The OP didn't choose the wrong approach/algorithm, they merely failed to implement it adequately. – DarthGizka May 09 '16 at 15:57