Recursive prime factor algorithm in java

I am trying to implement a simple algorithm in java for finding all prime number factors of an integer passed by parameter:

private static ArrayList<Integer> lstPrime= new ArrayList<Integer>();

    public static ArrayList primeFactors(int n) {

       开发者_开发问答 if (isPrime(n)) 
        {
            lstPrime.add(n);
        }

        for (int i=2; i<=(Math.sqrt(n)); i++)
        {
            if (n % i == 0)
            {
                lstPrime.addAll(primeFactors(n/i));
                return lstPrime;
            }
        }
        return lstPrime;
    }

Funny thing is that if I pass 81 as n, the result will be : 3, 3, 3, 3, 3, 3, 3, 3 while it SHOULD be : 3, 3, 3, 3 (3^4=81)

Perhaps a little more complex, but it works so far, and it uses probably the fastest (and smallest) prime number generator I could find in Java.

First, we got the prime number generator, needed to test if a value is prime. We use a generator (this one is 10x faster than the naïve method) so we use a cached list :

/**
 * Sieve of Sundaram prime number generator
 * Implementation following the Sieve of Sundaram to generate prime numbers 
 * 
 * @see http://en.wikipedia.org/wiki/Sieve_of_Sundaram
 */
static public class SundaramSievePrimeGenerator {
   public String getName() { return "Sieve of Sundaram generator"; }
   public int[] findPrimes(int max) {
      int n = max/2;
      boolean[] isPrime = new boolean[max];

      Arrays.fill(isPrime, true);

      for (int i=1; i<n; i++) {
         for (int j=i; j<=(n-i)/(2*i+1); j++) {
            isPrime[i+j+2*i*j] = false;
         }
      }

      int[] primes = new int[max];
      int found = 0;
      if (max > 2) {
         primes[found++] = 2;
      }
      for (int i=1; i<n; i++) {
         if (isPrime[i]) {
            primes[found++] = i*2+1;
         }
      }

      return Arrays.copyOf(primes, found);
   }
}

Then we have the two methods needed to actually get the list of prime factor for n :

/**
 * Reuse an instance of the SundaramSievePrimeGenerator
 */
static public List<Integer> findPrimeFactors(int n, SundaramSievePrimeGenerator g) {
   ArrayList<Integer> primeFactors = new ArrayList<Integer>();

   int[] primes = g.findPrimes(n+1);
   int v;

   // debug
   //System.out.print("** primes found : ");
   //for (int a : primes) {
   //   System.out.print(" " + a);
   //}
   //System.out.println();

   if (primes[primes.length-1] == n) {
      primeFactors.add(n);
   } else {

      int max = primes.length - 1;

      for (int i=max; i>=0; i--) {
         primeFactors.add(primes[i]);
         if (testPrimeFactor(n, primes[i], primes, i, primeFactors)) {
            break;  // we found our solution
         }
         primeFactors.clear();
      }
   }

   return primeFactors;
}

/**
 * Recursive method initially called by findPrimeFactors(n, g)
 */
static private boolean testPrimeFactor(int n, int v, int[] primes, int index, List<Integer> factors) {
   int v2 = v * primes[index];

   if (v2 == n) {
      factors.add(primes[index]);
      return true;
   } else if (v2 > n) {
      if (index > 0) {
         return testPrimeFactor(n, v, primes, index-1, factors);
      } else {
         return false;
      }
   } else {
      while (index > 0) {
         factors.add(primes[index]);

         if (testPrimeFactor(n, v2, primes, index, factors)) {
            return true;
         }

         factors.remove(factors.size()-1);   // no good, remove added prime
         v2 = v * primes[--index];
      }
      return false;   // at this point, we are still below n... so no good
   }
}

And finally, our test case :

int n = 1025;
SundaramSievePrimeGenerator generator = new SundaramSievePrimeGenerator();

List<Integer> factors = findPrimeFactors(n, generator);

if (factors.isEmpty()) {
   System.out.println("No prime factors found for " + n);
} else {
   System.out.println(n + " is composed of " + factors.size() + " prime factors");
   int v = 1;
   for (int i : factors) {
      v *= i;
      System.out.print(" " + i);
   }
   System.out.println(" = " + v);
}

For example, this code above will produce :

1025 is composed of 3 prime factors
 41 5 5 = 1025

And changing n = 81 will produce the desired output of

81 is composed of 4 prime factors
 3 3 3 3 = 81

the problem is your recursive implementation. use this:

public static ArrayList primeFactors(int n){
    if (isPrime(n))
    {
        list.add(n);
        return list;
    }
    int i = 1;
    while(true){
        if (n % (i+=2) == 0){
            if (isPrime(i))
            {
                n = n / i;
                list.add(i);
                i = 1;
            }
        }
        if (i> Math.sqrt(n))
            break;
    }
    list.add(n);
    return list;
}

public static void primeFactorsOf( int n ) 
{
    int i;

    if( isPrime( n ) )
        System.out.println( n +". " );
    else
    {
        for( i = 2; i < n; i ++ )
        {
            if( isPrime( i ) && n % i == 0 )
            {
                System.out.print( i +", " );
                n = n/i;
                primeFactorsOf( n );
            }
        }
    }
}

public static boolean isPrime( int n )
{
    int i;

    if( n < 2 )
        return false;
    else
    {
        for( i = 2; i < n; i += 1 )
        {
            if( n % i == 0 ) 
                return false;
        }
    }    

    return true;
}

OK! I think I have solved my problem, except for the fact that the ArrayList is declared outside the recursive function. I cannot imagine any other way of treating the list because since this is a recursive function, if the list would be declared inside the function, it would be instantiated again and again each time recursion occurs. Here is what I have so far, feel free to criticize:

public static ArrayList<Integer> list = new ArrayList<Integer>();

public static void primeFactors(int n) {
    if (isPrime(n)) 
    {
        list.add(n);
        return;
    }

    int i = 2;
    while (i < n/2)
    {
        if (n % i == 0)
        {
             if (isPrime(i))
             {
                 primeFactors(n/i);
                 list.add(i);
                 return;
             } 
        }
        i++;
    }
    return;
}

For example, this code will produce: 3,3,3,3 for primeFactors(81) and 5,3,2,2 for primeFactors(60) and so on...

public static void primeFactorsOf( int n )
    {
        int i = 2;
        boolean isFactor = false;

        if( isPrime( n ) )
            System.out.println( n+"." );
        else 
        {
            while( !isFactor )
            {
                if( ( n % i == 0 ) && ( isPrime( i ) ) )
                {
                    System.out.print( i +", " );
                    primeFactorsOf( n / i );
                    isFactor = true;
                }
                else
                    i ++;
            }
        }
    }

public static String soNguyenTo(int x, int i) {
    if (i == 1 && x > 1) {
        return "là số nguyen tố";
    }
    if (x == 0 || x % i == 0) {
        return "không phải là số nguyên tố";
    } else {
        return soNguyenTo(x, i - 1);
    }
}

public static void main(String[] args) {
    input = new Scanner(System.in);
    System.out.println("nhập số bất kỳ");
    int i = input.nextInt();
    System.out.println(i + ": " + soNguyenTo(i, (int) Math.sqrt(i)));

}

Edit: There is a design flaw. Why do you have to return the list. It's defined outside the function body and would get updated for each factor found. Therefore, no need to return it.

My Java is rusty so you'll have to add arrays/lists/vectors yourself, but this seems simpler than all the other solutions so far.

public static void primeFactors(int n) 
{
  for (int i = 2; i < n; i++)
    if (n % i == 0)
    {
      primeFactors(n / i);
      primeFactors(i);
    }

  System.out.println(n);
}