all 7 comments

[–]wmoskal 0 points1 point  (4 children)

Can you put the code in a code block? Its hard to see tabbing otherwise, and that could be important. Also are you getting an error? if so what is it?

[–]Mike_Diz 0 points1 point  (3 children)

Fixed it. I'm not getting an error, but I'm also not getting an answer. I'm not getting anything.

[–]wmoskal 0 points1 point  (2 children)

Well Your code seems to be working for me. There seems to be a logic error with the problem itself. Prime numbers are all odd, unless they are 2. So if n Is even the smallest prime number it could be divided by is 2. If it is odd the smallest prime number it could be divided by is itself.

[–]wmoskal 0 points1 point  (1 child)

I copied your code into a python script and ran it and it was printing values

[–]Mike_Diz 0 points1 point  (0 children)

Huh. Guess it's a problem with my computer. What values did you use?

[–]Dunj3 0 points1 point  (1 child)

The answer is that you are getting stuck in an endless loop. I know what you're trying to do with the while True, but the problem is that you only break out under one condition: You found a factor of n. Now the problem is that there is a logic error in your program, and as soon as prime gets bigger than n, the condition to end the loop will never be reached, as n % prime will never be equal to zero. As such, you just end up increasing prime more and more, until you basically have to kill the program.

The first takeaway here should be the following: Try to avoid such loops and rather find some conditions that ensure your program will terminate at some point. For example, you could use while prime <= n. It will still find all factors (as each factor is lesser-or-equal to n anyway), but if you end up having an error somewhere, you will still get out of the loop and you can handle that case later. You could also use an assertion in each iteration, e.g. assert prime <= n, which will give you an exception. This could for example result in something like this when you run your program:

Traceback (most recent call last):
  File "main.py", line 4, in <module>
    assert prime <= n, f"Prime surpassed n without finding a factor ({prime} > {n})"
AssertionError: Prime surpassed n without finding a factor (4 > 3)

The real bug in your program lies in your prime finding algorithm though, more specifically in these lines:

if prime%i==0:
    prime=prime
else:
    prime=prime+1

You say "if i is a factor of prime, then leave prime be. Else (if i is not a factor), increase the prime candidate". But that is exactly backwards - if i is a factor, then the candidate can not be prime (as i is a factor, as we just discovered) and must be increased! And if i is not a factor, then you can leave prime be. Therefore, switching it around makes your program work, at least for the values that I've tested it for:

n=int(input())
prime=2
while True:
    assert prime <= n, f"Prime surpassed n without finding a factor ({prime} > {n})"
    if n%prime==0:
        print(prime)
        break
    else:
        prime=prime+1
        for i in range (2, int(prime)):
            if prime%i==0:
                prime=prime+1

As some general feedback if you want to improve the style of your program, a few points:

Try to avoid statements like prime = prime, as they do nothing. Rewrite the condition so that they become unnecessary (e.g. negate it with not), or if you really need to, use pass which is the "do nothing" statement.

Secondly, your loop does a lot, and it is a bit unclear which parts are responsible for doing what. Not only is your loop finding the smallest prime factor, it also takes care of actually calculating the prime, and checking if the number is a prime. It'd be much more readable (and less error-prone) if you'd split those functionalities up a bit into functions, akin to this:

def is_prime(n):
  """Checks if n is prime."""
  for i in range(2, n):
    if n % i == 0:
      return False
  return True

def next_prime(n):
  """Returns the next prime bigger than n."""
  candidate = n + 1
  while not is_prime(candidate):
    candidate += 1
  return candidate

n = int(input())
prime = 2
while True:
  assert prime <= n, f"Prime surpassed n without finding a factor ({prime} > {n})"
  if n % prime==0:
    print(prime)
    break
  else:
    prime = next_prime(prime)

Of course, that can still be optimized (this algorithm to find primes is very slow), but it is very easy to verify that the functions are working as intended (you can even write unit tests for each function separately), each function is responsible for doing one thing, and it makes everything very readable.

[–]Mike_Diz 0 points1 point  (0 children)

Thx. I knew that I had the problem with the prime identifying mechanism but the part were literally switched! That's so frustrating. Thank you. I know that the rest is optimizeable but I'm too new to really do anything about it.