PHP, Generating Primes, What's Wrong

What's wrong with t开发者_如何学运维his? The conditional statement looks solid enough, yet all numbers are included

function generate_primes(){
   $max = 100;
   $primes = array();

   for($current_pointer = 1; $current_pointer <= $max; $current_pointer++){

        for($divider = 1; $divider <= $current_pointer; $divider++){
        //if(in_array($divider, $primes)){
        if(($current_pointer % $divider === 0) && ($divider !== 1) && ($divider ===    $current_pointer)){
           $primes[] = $current_pointer;
           }
//}

        }

    }
print_r($primes);

}
generate_primes();

You need to deal with the case that your divider does divide evenly into your current_pointer but the divider doesn't equal the current pointer. In that case, you need to jump out of the loop (i.e. you've found something that divides evenly, so the number isn't prime). As written, all loops eventually hit the case of dividing the number by itself, so all numbers succeed.

In other words, you're trying to test for the FIRST successful divider being the number itself, but to do that, you have to stop trying when you hit a different successful divider.

    if(($current_pointer % $divider === 0) && ($divider !== 1)){
         if ($divider === $current_pointer)
            $primes[] = $current_pointer;
         } else {
            continue; // the continue makes you stop testing the current pointer and go on to the next
         }

Re-think your algorithm. A prime number is a natural number > 1 that is not evenly divisible by any other natural number except itself and 1. So, if $current_pointer % $divider === 0, the number in question can't be a prime number, becauseit is divisible by $divider. So why do you add it to the array?

Edit in response to comments: To clarify, this means that you have to check for all possible divisors (from 2 to the number itself -1) and make sure that not a single of them divides the number in question without a remainder. In and only this case the number is a prime.

You can optimize this algorithm slightly (i.e. the root of your number can be the upper bound for the inner loop), but first try to get things working in the basic case.

I believe your last condition should be:

$divider !== $current_pointer

Two wrong things: 1) $divider === $current_pointer) should be $divider != $current_pointer) and 2) you are adding the number to the $primes array after each and every iteration.

Suggestions: 1) start the second loop by 2, since you want $divider not to be 1 in the first place, and 2) make $divider < $current_pointer (instead of <=), since you also want $divider not to be itself.

Try this:

function generate_primes(){    
    $max = 100;    
    $primes = array();    

    for($current_pointer = 1; $current_pointer <= $max; $current_pointer++){   

        $prime = true;  

        for($divider = 2; $divider < $current_pointer; $divider++){         

            if($current_pointer % $divider == 0) $prime = false;

        } 
        if($prime == true) array_push($primes, $current_pointer);
    } 
    print_r($primes);  
} 

generate_primes();

First off, your loop is a bit funny ;) If you want to exclude 1, don't start with 1

Second, I don't understand the last clause of your conditional. You are saying you will only consider it a prime if the divider is === to current pointer. I think you mean != to current pointer.

Look at your if statement. It essentially says:

Mark a number as prime if:

1. current_pointer equals 0.
2. divider doesn't equal 0.
3. divider equals current_pointer.

The only time all 3 of those conditionals will be true is when the final conditional is true (divider equals current_pointer.)

If the current pointer is 6, then when the divider equals 6, you'll store it as a prime (obviously not true.)

See this example as a simple (although expensive implementation) function for testing if a number is prime (you could easily use the logic to make a generator.)
Prime number checker

I'd suggest you look up the Sieve of Eratosthene algorithm. It's for getting a list of primes, and is very fast compared to the brute force approach you are taking. While your code will run fine for smaller numbers, you'll find as you try to, for example, get all primes < 10,000,000 it will take a long time.

The problem is that

if( ($current_pointer % $divider === 0) && 
    ($divider !== 1) && 
    ($divider === $current_pointer)){

will always be true when $divider == $current_pointer

A better approach is to use a is_prime flag initiated to true. Set it to false if any number smaller than it divides it.

A further enhancement would be to add the number two, and then only check non even numbers.

   function generate_primes($max = 100){    
        $primes = array(2);    

        for($current_pointer = 3; $current_pointer <= $max; $current_pointer += 2){   
            if(is_odd_number_a_prime($current_pointer)){
                $primes[] = $current_pointer;
        } 
        print_r($primes);  
        return $primes
    } 

    function is_odd_number_a_prime($n){
      if(is_int($n)){
            for($divider = 3; $divider < $n; $divider +=2 ){             
                if($current_pointer % $n == 0){
                    return false;
                }
            } 

      }else{
        return false;
      }     
      return true;
    }

   $primes = generate_primes();

You could further enhance things by using a sieve.