0
\$\begingroup\$

Here is my code of project Euler #7:

from math import *
import time
start = time.time()
def isPrime(n):
    prime_fst = [2,3,5,7]
    num = round(sqrt(n))
    j=0
    while num>7:
        if num%2!=0 and num%3!=0 and num%5!=0 and num%7!=0:
            prime_fst.append(num)
        num-=1
    prime_fst.sort()
    while j < len(prime_fst):
        for i in prime_fst:
            if i!=prime_fst[j]:
                if i%prime_fst[j]==0:
                    prime_fst.remove(i)
        j+=1
    for t in prime_fst:
        if n%t==0:
            k=n%t
            return False
    return True

x=int(input())
l=1
res = [2,3,5,7]
while len(res)!=x:
    if l!=1:
        if isPrime(l):
            res.append(l)
    l+=2
print(res[-1])
end = time.time()
print(end-start)

Can you recommend what I should do with algorithm skills? How can I think effectively? Will skills develop with time? All your recommendations and help to speed up this code by explaining?

\$\endgroup\$

closed as unclear what you're asking by πάντα ῥεῖ, Stephen Rauch, hjpotter92, Heslacher, Ludisposed Oct 8 '18 at 8:48

Please clarify your specific problem or add additional details to highlight exactly what you need. As it's currently written, it’s hard to tell exactly what you're asking. See the How to Ask page for help clarifying this question. If this question can be reworded to fit the rules in the help center, please edit the question.

  • 1
    \$\begingroup\$ To respond to the "too broad" close vote you could try removing the tangential questions like "How to think effectively?" - those questions don't belong on this site. I'm not sure why someone considered this question unclear. \$\endgroup\$ – l0b0 Oct 6 '18 at 21:26
1
\$\begingroup\$

Some recommendations:

  • Running your code through a linter like pycodestyle, then learning what the messages mean and applying them to your code is going to make your code more idiomatic, and therefore easier to understand for Python veterans.
  • I would use an external command like time my_script.py to time the execution. This will shorten your code a fair bit (and besides, you would never put timing code into production code). Sure, it's a little bit less accurate, but for Euler Project questions you just want a ballpark estimate.
  • Use argparse rather than input(). That makes your scripts, well, scriptable, so that they can be used easier within the context of a bigger project, and they can be re-run quicker on the command line.
  • The only things on the top level of your script should be a shebang line like #!/usr/bin/env python3, imports, class/function definitions, and the following:

    if __name__ == '__main__':
        main()
    

    This way, anything declared in the script can be imported by other scripts without running the algorithm.

  • Naming is one of the most difficult but also most useful skills in programming. If you can think of any better names for n, t, num, j, etc, the program will be much easier to understand for someone without all the context you have.
  • You hard code the list [2, 3, 5, 7] three times in your code, but only once (at most) should be necessary.
  • On a related note, starting with a list of the four first primes is very arbitrary. Why not the first million? Or the first one?
  • isPrime (or is_prime as it should be in idiomatic Python) is a good name, but there is no explanation for which algorithm you're using to check primality. You can introduce the algorithm name in one of several ways:
    • As a comment.
    • By refactoring the entire contents of isPrime into a separate method and naming that according to the algorithm. This only really makes sense if either isPrime ends up containing something more than just the call to the other function (a "vacuous" function) or if you intend to implement multiple algorithms in your code and swap them out dynamically (for example, using the fastest algorithm based on the size of the numbers you're dealing with).
    • More complex means like a dispatcher, but this is not applicable in this situation.
  • Rather than round(sqrt(n)) you can use floor(sqrt(n)), it'll be faster half the time.
  • Every time you run isPrime you build up a list of primes, but it goes out of context and has to be recomputed for every number. Your code will be much faster for long lists of input if you save all the primes calculated so far for subsequent runs. For example, you could create an ErathostenesSieve class which has a field sieve which contains the primes detected so far, and it could even be a generator from which you can take an arbitrary number of primes.
  • If you start with l = 2 or even l = res[-1] you can remove if l!=1:.
  • Do not append to prime_fst until you're sure the number is prime. That way you won't ever have to sort the list or remove from it.

By the way: Don't be discouraged by the amount of suggestions! IMO this is a very good start, and learning programming is an infinite journey which can be incredibly rewarding.

\$\endgroup\$

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