views:

53

answers:

3
public class prime
{
   public static void main(String[] args)
    {
     long thing = 600851475143L;
     for(long i = 300425737571L ; i == 0 ; i-- ){
     if (thing % i == 0)
     {
       long answer = i;
       System.out.println(answer);
       break;
     }
     }

    }

}

This is the code I currently have, however I have been running it in DrJava for a few minutes and it has returned no results. I'm guessing there are roughly a million ways my code can be optimised though ; would anyone be able to give me some tips ?

Trying to work through a few programming problems today and this one is causing me some trouble.

Thanks a lot :)

+2  A: 

No, it's terminating right away, since

i == 0

will not hold on the first iteration.

You probably wanted to write something like this:

public class Test {
    public static void main(String[] args) {
        long thing = 600851475143L;
        for (long i = 16857 /* 300425737571L */; i > 0; i--) {
            if (thing % i == 0) {
                long answer = i;

                // Print the largest prime factor, then break loop (and quit)
                System.out.println(answer);
                break;
            }
        }
    }
}

This naive factorization method however is extremely inefficient. Since the factorization of 600851475143 is 71 * 839 * 1471 * 6857 you would have to iterate from 300425737571 to 6857 and do a modulo each time. There are many, not that complicated methods that would solve factorization for longs in an instant.

aioobe
So it should be i==0L?
+1 for explaining why this is inefficient, though
Thilo
I see...so can you offer any pointers for devising a more optimised solution please ? :) Thank you.
@Thilo, I don't get your comment. "It would only break if the condition was true.", but `for (; true; );` is an infinite loop... that is, it will *not* break if the condition is true.
aioobe
@user476033: sure, [here](http://www.cs.princeton.edu/introcs/13flow/Factors.java.html) for instance.
aioobe
@aioobe: Ohh, that condition... I looked at the wrong code. Never mind.
Thilo
+2  A: 

For calculating prime factors on not extremely big values, it would be much more efficient to generate primes up to square root of "thing", and then try them one by one.

Primes generation may be done:

Kel
+3  A: 

You only need iterate down to sqrt(thing), though.

And in general it's going to be quicker to iterate starting from 2, since half the numbers will have a factor of two (and 1/3 a factor of 3 etc.

You're also breaking only on the first factor so will miss any others

 long thing = 600851475143L;
 for(long i = 0; i < 300425737571L ; i++ ){
     if (i * i > thing) {
       break;
     }
     if (thing % i == 0) {
       long answer = i;
       System.out.println(answer);
       break;
     }
 }
  • more sophisticated methods are available as aioobe says
Paul
`300425737571L ; ; i++ ){` one `;` too many!
Ishtar
Thanks. Fixed it.
Paul