I tried to make an is_Prime() function in PHP. What do you think about this code?

<?php
function is_Prime($x)
{
  if($x <= 1)
  {
    echo $x. " is not a prime number.</br>";
    return false;
  }
  $data = array();
  for ($i=1;$i<=$x;$i++) 
  { 
    if(is_integer($x/$i))
    {
        array_push($data, ($x/$i));
    }
  }
  if(count($data)>2)
  {
    echo $x." is not a prime number</br>";
  }
  else
  {
    echo $x. " is a prime number</br>";
  }
}
?>

How to use?

for($i=0;$i<=100;$i++)
{
  is_Prime($i);
}
share|improve this question
2  
Kind of strange naming of your function. Either use isPrime() or is_prime(), mixing camelCase and snake_case looks odd – holroy Nov 13 '15 at 3:32
    
A related approach is the Sieve_of_Eratosthenes – Tensibai Nov 13 '15 at 15:32

All primes end in either 1, 3, 7, or 9. You could do a check up front to make sure the number satisfies that requirement.

Second, a number is prime if it has no prime factors besides itself and 1. Therefore you should start you checking loop at 3, not at 1. Since you only need to check against primes, you could start the loop at 3, increment it by 10, and check $x/$i, $x/($i+4), $x/($i+6), and $x/($i+8). That would be 4 divide operations instead of the 10 you are presently doing.

Third, you only have to check possible factors up to the square root of the number you are checking, so figure out the sqrt($x) and make that the end of the loop. You are also putting all the factors you find into an array, which is not necessary if you are simply trying to see if the number is prime.

share|improve this answer
    
To achieve the second point, there is a trick at end of this answer. Start with $i=5 and $add=2. Then when incrementing $i you do $i += $add and $add = 6 - $add. This will alternate the increment of $i between 2 and 4, which skips all numbers divisible by 2, 3, and 6. Changing the increment, would allow you to keep the division test as is, as you skip the unneccessary $i . – holroy Nov 13 '15 at 3:30
    
thanks for your advice – Zaki Sulistya Nov 13 '15 at 9:50

Here's my version of your code and an explanation of what I did and why.

<?php

function isPrime($number)
{
    if ($number <= 1) {
        return false;
    }

    $isPrime = true;
    for ($i = 2; $i < $number ; $i++) {
        if (is_integer($number / $i)) {
            $isPrime = false;
            break;
        }
    }

    return $isPrime;
}

for ($i = 0; $i <= 100; $i++) {
    echo $i;
    echo isPrime($i) ? " is a prime number" : " isn't a prime number";
    echo "<br>";
}

?>
  1. Use four spaces for indentation as per PSR-2. This will make your code more readable.
  2. Stick to camelCase for your function name as per PSR-1. This will make your code more readable.
  3. Make your function return true or false so you can use your code in multiple places without having to change what it outputs. This will make your code reusable.
  4. Make your function properties more descriptive. This will make your code more readable.
  5. Make your for loop start at two as we have establish at the top of the function that the number isn't equal to one or less. This will increase performance of your code.
  6. Make your loop stop at one number before the number in question as we know it will be divisible by itself. This will increase performance of your code.
  7. Make your loop exit as soon as it hits an integer. We have already excluded the number itself and the number one so we can be sure it isn't a prime number. This will increase performance of your code.
share|improve this answer
1  
The $isPrime can be dropped, just return true or false. And related to (6) the loop should be terminated at around sqrt(n) not n-1, that would half the execution time... – holroy Nov 13 '15 at 16:59
    
@holroy Thanks for the critique! Point taken regarding $isPrime, what's the reason behind stopping at the square root of the number? – OhFiddyYouSoWiddy Nov 13 '15 at 17:24
1  
You've already compared the other side. Take 21 where sqrt(21)<5, when you've tested 3, there is no need to test for 7... – holroy Nov 13 '15 at 18:02
1  
thanks for your advice – Zaki Sulistya Nov 17 '15 at 10:25

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.