Timeline for answer to Rate my first python prime checker by Adam Taylor
Current License: CC BY-SA 4.0
Post Revisions
26 events
| when toggle format | what | by | license | comment | |
|---|---|---|---|---|---|
| Feb 20, 2020 at 7:27 | comment | added | user218840 | Remember comments are not a replacement for good variable names. Naming things like "num", "sqrt", "x" doesn't help much. Consider changing them to variable names that accurately represent what they are; eg "num" to "prime_candidate", "sqrt" to "maximum_factor", "x" to "i". Remove gratuitous comments like "# no factor found, loop falls through" - good code comments itself, when that fails rely on comments. If the naming and logic is improved then it becomes obvious that when all factors are checked, the loop will end and the number must be a prime. | |
| Feb 19, 2020 at 22:14 | comment | added | pts |
This is how to get square root without loss of precision: import decimal; c = decimal.Context(); c.prec = len(str(num)); print(int(c.sqrt(num))). Anything using math.sqrt or math.floor or Decimal without Context is inaccurate.
|
|
| Feb 19, 2020 at 22:08 | comment | added | pts |
@oobug: int(Decimal(num).sqrt()) is also incorrect, e.g. it's 1 less than the correct result 10000000000000000000000000001 for num 100000000000000000000000000020000000000000000000000000001.
|
|
| Feb 19, 2020 at 22:00 | comment | added | pts |
@oobug: Actually, int(math.floor(Decimal(num).sqrt())) also returns 1 less. To get the correct result, drop the math.floor.
|
|
| Feb 19, 2020 at 21:23 | comment | added | oobug |
@pts Yes, that's true. You would get more consistently accurate results by importing the Decimal class from the decimal module and using int(math.floor(Decimal(num).sqrt()))
|
|
| Feb 19, 2020 at 20:33 | comment | added | pts |
FYI int(math.sqrt(num)) and int(math.floor(math.sqrt(num))) are both inaccurate. For example, the floor of the square root of 81129638414606699710187514626049 is 9007199254740993, but these formulas return 1 less than that.
|
|
| Feb 19, 2020 at 10:18 | vote | accept | Zakaria | ||
| Feb 19, 2020 at 0:55 | history | edited | Adam Taylor | CC BY-SA 4.0 |
make terminology clearer
|
| Feb 18, 2020 at 21:29 | comment | added | Adam Taylor | Nice optimisations AJNeufeld! I'll make sure to remember those for future :) | |
| Feb 18, 2020 at 21:25 | history | edited | Adam Taylor | CC BY-SA 4.0 |
added odd numbers only optimisation
|
| Feb 18, 2020 at 21:25 | comment | added | AJNeufeld |
Your for ... if ... break could easily be turned into an if any statement, with the same short-circuit behaviour. Eg, something like: if any(num % x == 0 for x in range(3, math.isqrt(num) + 1), 2): print("Not prime") else: print("Prime")
|
|
| Feb 18, 2020 at 21:24 | comment | added | AJNeufeld | You should use a step-size of two in your loop, since you've already tested even numbers with the first if statement. | |
| Feb 18, 2020 at 21:13 | comment | added | AJNeufeld |
Python 3.8 include math.isqrt(num) which avoids the need for things like int(math.floor(math.sqrt(num)))
|
|
| Feb 18, 2020 at 21:08 | history | edited | Adam Taylor | CC BY-SA 4.0 |
fix bug with 9 being returned as prime
|
| Feb 18, 2020 at 21:00 | comment | added | Adam Taylor | Thanks all for the comments! Thanks gnasher729 for spotting the bug - it's fixed | |
| S Feb 18, 2020 at 20:59 | history | edited | Adam Taylor | CC BY-SA 4.0 |
fixed bug, added additional points
|
| Feb 18, 2020 at 20:36 | comment | added | TemporalWolf | Your code also shows an optimization with sqrt; it would improve your answer to edit and call out this optimization since no one else has suggested it and it will substantially reduce the search space. | |
| Feb 18, 2020 at 18:57 | review | Suggested edits | |||
| S Feb 18, 2020 at 20:59 | |||||
| Feb 18, 2020 at 16:17 | comment | added | Vogel612 | Welcome to Code Review! I've removed the salutation from this post, since this site is trying to focus only on the contents of a given post. Upon reading around, you might notice that little to no other posts have salutations in them :) In addition to that I reformulated the way you refer to AJNeufeld's answer, (notably removing the word "example"), mostly to make it "scan" easier without the salutation. | |
| Feb 18, 2020 at 16:14 | history | edited | Vogel612 | CC BY-SA 4.0 |
removed salutations and reformulated the attribution to AJNeufeld to scan a bit easier.
|
| Feb 18, 2020 at 16:12 | history | notice removed | Vogel612 | ||
| Feb 18, 2020 at 16:03 | comment | added | gnasher729 | Prints the wrong result when num = 2. | |
| Feb 18, 2020 at 16:01 | history | edited | Adam Taylor | CC BY-SA 4.0 |
updated to add points
|
| Feb 18, 2020 at 15:47 | history | notice added | Vogel612 | Insufficient justification | |
| Feb 18, 2020 at 15:35 | review | First posts | |||
| Feb 18, 2020 at 15:47 | |||||
| Feb 18, 2020 at 15:31 | history | answered | Adam Taylor | CC BY-SA 4.0 |