2
\$\begingroup\$

This Python code looks messy to me. Am I doing any wrong style, or is this how Python looks like?

def is_prime(number):
    """check whether the @param number is a prime number"""
    is_prime = True
    for i in range(2, (number - 1)):
        is_prime = ((number % i) != 0)
        if not is_prime:
            return is_prime
    return is_prime

This Java code looks more clean, maybe due to a lot of empty lines generated due to the { } brackets:

private boolean isPrime(int toCheck)
{
    boolean isPrime = true;

    for (int i = 2; i < toCheck; i++)
    {
        isPrime = ((toCheck % i) != 0);
        if (!isPrime)
        {
            return isPrime;
        }
    }

    return isPrime;
}

Python version 2.6.

\$\endgroup\$
0
4
\$\begingroup\$

This is a pretty literal translation of the algorithm. Let's first review the code itself and then check how we could reimagine it in python:

  • I'm not sure why there is a declaration for is_prime or isPrime respectively. It can be inlined, making the code look as follows:

    for i in range(2, (number - 1)):
        if (number % i) == 0:
            return False
    return True
    

    This uses some minor formatting simplifications, like dropping extraneous parentheses.

  • The next step is then to actually optimize the program.
    In this instance, using xrange over range is preferred, since you don't need to store the whole list of numbers before iterating over it, which is what range does. Note that this only applies for python 2.x, since xrange is not available for python 3.x

  • Mathematically it's proven, that the first prime divisor of a non-prime number appears before (or athat tip to jwodder) the square root of said number. This can vastly reduce your searchspace:

    for i in xrange(2, math.sqrt(number) + 1):
    
  • Peilonrayz correctly added in a comment that the full code can be reformulated as a generator-comprehension using all, which is basically the "most python-y" implementation I've seen yet:

    all(number % i for i in range(2, int(number ** 0.5) + 1))
    

Overall the code is stylistically fine. The Docstring could use a bit of love and the algorithm chosen doesn't lend itself well to python's idea of generators (research this concept, it's key to writing idiomatic and fast python).

Finally: Your java code looks messy to me, since it doesn't conform to the standard egyptian bracing style and is very spacious vertically. In the end this seems to boil down to personal preference of vertical space in code.

\$\endgroup\$
4
  • \$\begingroup\$ over time it has been glued in my mind, the more empty vertical gaps the better. Now I understand. thank you \$\endgroup\$ – Srinath Ganesh Nov 12 '17 at 12:44
  • 1
    \$\begingroup\$ Nice answer, however I'd recommend using a comprehension rather a generator. Unless that's a generator comprehension. This could make the code as simple as all(number % i for i in range(2, int(number ** 0.5))) \$\endgroup\$ – Peilonrayz Nov 12 '17 at 19:30
  • 1
    \$\begingroup\$ "the first prime divisor of a non-prime number appears before the square root" — That should be "before or at", and the upper bound for the xrange should be changed to math.sqrt(number)+1; consider what would happen if number were, say, 25. \$\endgroup\$ – jwodder Nov 12 '17 at 20:39
  • \$\begingroup\$ 1 is not a prime number \$\endgroup\$ – Roland Illig Nov 12 '17 at 22:00
1
\$\begingroup\$

If I’m understanding your question correctly, you’re just a little disoriented by Python’s code block style, which is only spaces without the curly braces {} that are found in many other languages.

I also found this disorienting initially, but you quickly get used to it.

I can recommend using an editor / IDE that nicely supports Python - PyCharm is a good one.

Otherwise unless you’re actually adding a multline comment, you can replace the “”” with a single leading # for a single line comment which might be a bit easier to read until you get used to the “”” style.

\$\endgroup\$
1
  • 3
    \$\begingroup\$ While this is helpful advice, it is not a review. On Code Review, all answers should contain at least a review. \$\endgroup\$ – Mast Nov 12 '17 at 14:06
0
\$\begingroup\$

While you tried to do a literal translation you did an error (that has no effect in your example)

for (int i = 2; i < toCheck; i++)

does not translate to

for i in range(2, (number - 1)):

but

for i in range(2, number):

A Python range is a right-open interval so range(0,number) gives a sequence with length number from 0 to number-1.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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