Project Euler #10 Java solution not working

I'm trying to find the sum of the prime numbers < 2,000,000. This is my solution in Java but I can't seem get the correct answer. Please give so开发者_Go百科me input on what could be wrong and general advice on the code is appreciated.

Printing 'sum' gives: 1308111344, which is incorrect.

Edit: Thanks for all the help. Changed int to long and < to <= and it worked flawlessly, except for being an inefficient way of finding prime numbers :)

/*
The sum of the primes below 10 is 2 + 3 + 5 + 7 = 17.
Find the sum of all the primes below two million.
*/
class Helper{
 public void run(){
  Integer sum = 0;
  for(int i = 2; i < 2000000; i++){
   if(isPrime(i))
    sum += i;   
  }
  System.out.println(sum);
 }

 private boolean isPrime(int nr){
  if(nr == 2)
   return true;
  else if(nr == 1)
   return false;
  if(nr % 2 == 0)
   return false;

  for(int i = 3; i < Math.sqrt(nr); i += 2){
   if(nr % i == 0)
    return false;
  }  
  return true;  
 }
}   
class Problem{
 public static void main(String[] args){
  Helper p = new Helper();
p.run();
}
}

The result will be too large to fit into an integer, so you are getting an overflow. Try using a BigInteger or a long instead. In this case a long is enough.

You're treating as prime those numbers that are only divisible by their square root (like 25). Instead of i < Math.sqrt(nr) try i <= Math.sqrt(nr).

That's a really inefficient way to find primes, incidentally.

Your isPrime doesn't work for squares. isPrime(9) returns true.

As already stated errors were two:

• you used an int that is not big enough to hold that sum.. you should have used a long
• you used < instead that <=, and it was a wrong guard for the cycle

Apart from that what you are doing is really inefficient, without going too deep inside this class of algorithms (like Miller-Rabin test) I would suggest you to take a look to the Sieve of Eratosthenes.. a really old approach that teaches how to treat a complex problem in a simple manner to improve elegance and efficiency with a trade-off of memory.

It's really cleaver: it keeps track of a boolean value for every prime up to your 2 millions that asserts if that number is prime or not. Then starting from the first prime it excludes all the successive numbers that are obtained by multiplying the prime it is analyzing for another number. Of couse more it goes and less numbers it will have to check (since it already excluded them)

Code is fair simple (just wrote it on the fly, didn't check it):

    boolean[] numbers = new boolean[2000000];
    long sum = 0;

    for (int i = 0; i < numbers.length; ++i)
        numbers[i] = true;

    for (int i = 2; i < numbers.length; ++i)
        if (!numbers[i])
            continue;
        else {
            int j = i + i;
            while (j < 2000000) {                   
                numbers[j] = false;
                j += i;
            }           
        }

    for (int i = 2; i < 2000000; ++i)
        sum += numbers[i] ? i : 0;

    System.out.println(sum);

Of course this approach is still unsuitable for high numbers (because it has to find all the previous primes anyway and because of memory) but it's a good example for starters to think about problems..

by using Sieve of Eratosthenes effectively, i solved the problem, here is my code

public class SumOfPrime {

    static void findSum()
    {
        long i=3;
        long sum=0;
        int count=0;
        boolean[] array = new boolean[2000000];
        for(long j=0;j<array.length;j++)
        {
         if((j&1)==0)
          array[(int)j]=false;   
         else    
         array[(int)j]=true;
        }
        array[1]=false;
        array[2]=true;
        for(;i<2000000;i+=2)
        { 
            if(array[(int)i] & isPrime(i))
            {   
                array[(int)i]=true;
                //Sieve of Eratosthenes
                for(long j=i+i;j<array.length;j+=i)
                    array[(int)j]=false;
            }
        }
        for(int j=0;j<array.length;j++)
        {
            if(array[j])
            {   
             //System.out.println(j);
             count++;   
             sum+=j;
            }
        }   
        System.out.println("Sum="+sum +" Count="+count);
    }
    public static boolean isPrime(long num)
    {
        boolean flag=false;
        long i=3;
        long limit=(long)Math.sqrt(num);
        for(;i<limit && !(flag);i+=2)
        {
            if(num%i==0)
            {
                flag=false;
                break;
            }   
        }
        if(i>=limit)
         flag=true;
        return flag;
    }

    public static void main(String args[])
    {
        long start=System.currentTimeMillis();
        findSum();
        long end=System.currentTimeMillis();
        System.out.println("Time for execution="+(end-start)+"ms");
    }

}

and the output is

Sum=142913828922 Count=148933
Time for execution=2360ms

if you have doubt, please do tell

Here is my solution

  public class ProjectEuler {

    public static boolean isPrime(int i) {
        if (i < 2) {
            return false;
        } else if (i % 2 == 0 && i != 2) {
            return false;
        } else {
            for (int j = 3; j <= Math.sqrt(i); j = j + 2) {
                if (i % j == 0) {
                    return false;
                }
            }

            return true;
        }
    }


    public static long sumOfAllPrime(int number){
        long sum = 2;

        for (int i = 3; i <= number; i += 2) {
            if (isPrime(i)) {
                sum += i;
            }
        }

        return sum;
    }

    /**
     * @param args
     */
    public static void main(String[] args) {
        System.out.println(sumOfAllPrime(2000000));
    }
}