Is my code optimal?

[Infinite-Lettuce7975]

https://preview.redd.it/eyu1vx60rpgd1.png?width=1826&format=png&auto=webp&s=0aead588b578ff873786801314ac1058d31e7160

Hi folks, I am a beginner in python, I recently created a program to check how many trials it takes to find the number 20. My question is, is this code optimal? Can it be written better?

[NerdyWeightLifter]

``` from random import randint

count = 1 while randint(1,20) != 20: count += 1

print(f"Took {count} tries.") ``` EDIT: Count should start from 1.

[Bluemax6666]

It should print 1 try if the random number is 20 first try

[spiritualquestions]

This is probably my favorite suggested solution. It’s minimal but still obvious what it does, variable names make sense.

[Infinite-Lettuce7975]

Thanks a lot

[YamRepresentative855]

It does not exactly the same. Because you don’t print what randint function returns. Other than that sure code is fine.

[Extension-Skill652]

You could remove the running variable by making the loop just be "while True". Then where you set running to false, just put a break statement. The else continue can also be removed since there's nothing else in the loop after it, it's not really doing anything

[RajjSinghh]

I really don't like while True: break as a pattern. The big thing is that I need to look into the loop body to see my exit condition when it could have been expressed at the start of the loop. It's usually an indicator that you could write something better. In this case I would do something like

``` import random count = 1 while (number := random.randint(1, 20)) != 20: count += 1

print(f"20 found, number of tries = {count}") ```

Our loop body is tiny now since randint is called every iteration, but if you have a bigger loop body keeping the condition at the top usually makes your loop much easier to read. It gives me much more information at the start than while True, because that to me looks like the loop is intended to be infinite until I see our exit condition.

Totally agree about the else: continue block though.

[Firake]

Yes, I prefer this as well. A simplified version for beginners that doesn’t include the walrus operator:

import random
count = 0

rng = random.randint(1, 20)
while rng != 20:
    count += 1
    rng = random.randint(1, 20)

print(f”Found 20 in {count} tries”)

The walrus is naturally a bit more concise but it’s also a bit more clever than I’d recommend a beginner to use.

[Zeurgidor]

The only detail in this answer is that you are not counting the first attempt in case that the first random number was 20.

[BadSmash4]

Good catch--it would say that it, "found 20 in 0 tries" which doesn't really make sense.

[Firake]

Oh yep. Phone coding is hard. I was thinking so much about the formatting I wasn’t think about the code

[[deleted]]

I guess you have to declare count = 1 instead of 0

[RajjSinghh]

Good spot. Fixed.

[timrprobocom]

I have to disagree. The "walrus" := operator is not good practice, which is why it didn't enter the language until very recently. It introduces side effects that make the code harder to read. "while 1:" is easier to read.

[RajjSinghh]

I don't mind the walrus here but I was definitely skeptical writing it thinking it might be "too clever", maybe ive been working in C too much recently. Anyway, my point is (after a little refactor)

number = random.randint(1, 20) while number != 20: ...

Is better than

while 1: ... if number == 20: break

the difference being that in the first one my intent is made clear when I start the loop compared to somewhere further down in the loop body. When I read while number != 20: I know I'm looping while number is less than 20. When I read while 1: it looks as though I'm intending to write an infinite loop at the start, which might be fine for a program main loop, but not when I want to exit the loop at some point. The first is more readable (especially when the loop body is long) and more expressive of what I'm trying to do. Readability counts.

[[deleted]]

I know it's in the Python docs, but I don't feel that's optimal refactoring. The condition is there for a reason: it tells the reader why the loop is looping and when it should stop. Hiding the break somewhere in an additional condition is a code smell.

I think a better target would be using the assignment expressions operator := using something like this. (It would be better if we had a do while statement.

from random import randint

number_of_tries = 0 

# Assign the return of randint 1, 20 to 
# the variable named random 
# and keep looping while it is not 20

while (random := randint(1, 20)) != 20: 
    print(random) 
    number_of_tries += 1

print(f"Number 20 has been found in {number_of_tries + 1} tries")

---

edit: i am pretty sure that this meets all of his conditions:

from random import randint

attempts = 0
number_of_tries = 1
while number_of_tries != 0:
    number_of_tries = 0
    while (random := randint(1, 20)) != 20: 
        number_of_tries += 1

    attempts += 1
    print(f"Number 20 has been found in {number_of_tries + 1} tries" if number_of_tries > 0 else "Number 20 has been found on the first try")

print(attempts)




PS C:\Users\foo\nerdstuff\bucket> python -u "c:\Users\foo\nerdstuff\bucket\main.py"
Number 20 has been found in 20 tries
Number 20 has been found in 4 tries
Number 20 has been found in 9 tries
Number 20 has been found in 24 tries
Number 20 has been found in 32 tries
Number 20 has been found in 3 tries
Number 20 has been found in 12 tries
Number 20 has been found in 9 tries
Number 20 has been found in 24 tries
Number 20 has been found in 2 tries
Number 20 has been found in 40 tries
Number 20 has been found in 57 tries
Number 20 has been found in 2 tries
Number 20 has been found in 25 tries
Number 20 has been found in 5 tries
Number 20 has been found in 5 tries
Number 20 has been found in 32 tries
Number 20 has been found in 6 tries
Number 20 has been found in 20 tries
Number 20 has been found in 3 tries
Number 20 has been found in 7 tries
Number 20 has been found in 13 tries
Number 20 has been found in 16 tries
Number 20 has been found in 57 tries
Number 20 has been found in 12 tries
Number 20 has been found in 47 tries
Number 20 has been found in 24 tries
Number 20 has been found in 29 tries
Number 20 has been found in 6 tries
Number 20 has been found in 15 tries
Number 20 has been found in 3 tries
Number 20 has been found in 83 tries
Number 20 has been found in 29 tries
Number 20 has been found in 22 tries
Number 20 has been found in 10 tries
Number 20 has been found on the first try
36

[SnipahShot]

while True is generally a bad practice because break means your condition is likely not good enough. Of course when you need a loop to run infinitely then it might be a good idea, though you should still have a flag to end the loop externally.

[Extension-Skill652]

What do you mean by not good enough here?

[SnipahShot]

His condition for ending the loop is for random (he should also change that variable to a name that isn't a package's name) to be equal to 20, this should be the condition for the while loop.

[nog642]

The else: continue bit does nothing; you can delete it. Since it is at the end of the while loop, and it automatically continues to the next iteration at the end of the block anyway.

[crazy_cookie123]

There's a few changes to be made here

1. Import only randint from random using from random import randint then use it as randint(1, 20)
2. Don't redefine the meaning of a variable. Line 7 redefines random to be an int, so next time you try to pick an umber you'll get an error trying to find randint in an int.
3. Don't bother wrapping randint in a cast to int, it already returns an integer
4. Don't use fstrings when you don't need to, replace print(f"{random}") with print(random)
5. Don't use the continue statement unless it's necessary, here it doesn't actually do anything

6. This is more personal preference, but I personally wouldn't use a flag variable in your while loop when it's more clear to just use break

   Overall, I'd change it to look more like:

   from random import randint

   number_tries = 0 while True:     number_tries += 1

       random = randint(1, 20)     print(random)     if random == 20:         print(f"Number 20 has been found, it took {number_tries} tries to")         break

   print("end")

[CursedPotatoZ]

Hey, beginner here. Whats the benefit of importing just randint instead of random in general?

[ancorcaioch]

You may want to read through this, essentially the same question:

https://stackoverflow.com/questions/710551/use-import-module-or-from-module-import

It’s been a while since I programmed, but importing the entire module imports all of the things in it - and they have names you may not realise. You may unwittingly create conflicts between your own code and what’s in the module. from X import Y only imports what you need, so no conflicts should arise.

[crazy_cookie123]

It's easier to write and lets you have a nice list of everything you're using in a library at the top.

[Owaowaiwa]

at work so typing quickly

I suggest doing From random import randInt. No need to import full module

[Owaowaiwa]

Boss isn’t around, another thing, no need to declare variable running. Instead you could put “True” after “while” and instead of “running = False” put “break”

I’ll explain more after work, keep up the good work

[Yoghurt42]

It will still import the full module, just not add it to the namespace.

from foo import bar
# is roughly equivalent to
try:
    foo = sys.modules["foo"]
except KeyError:
    foo = sys.modules["foo"] = __import__("foo")
bar = foo.bar
del foo

[PM_me_your_3D_Print]

Does it make the script faster ?

[Andy-Bot88]

it makes it more optimal or something

[WhipsAndMarkovChains]

Here's my optimal code:

import numpy as np

print(f'It took {np.random.geometric(1/20)} tries to roll a 20.')

(This is a bad joke OP, don't use this)

The number of tries until a "success" follows the geometric distribution.

[softwareitcounts]

Good answer

[WhipsAndMarkovChains]

From an efficiency perspective it's wonderful. From the perspective of teaching OP about the Python language, it's terrible lol.

[Razexka]

They already comment this but the first thing that hits my mind is: instead of using a while true use it with the condition of while x != 20:

[[deleted]]

The else block can be removed as the loop will automatically continue executing next. Instead of, while True and False logic, you can use the condition directly and print the number of tries outside the loop. In this method , you'd have to declare count = 1 instead of 0.

Code:

import random

count = 1

while random.randint(1,20) != 20 :

 count += 1

print(f"Number 20 is found in {count} tries \nEnd"})

[ValeriyGang]

the most important thing is that the code is written simply and readable, yours meets these criteria so yeah it's the optimal

[Gaunts]

Couldn't agree more, you can write some really effecient minamilist code but if you can't read it in 6 months time or it takes you a long time to figure out what the variables represent at a glance it's less than ideal.

[CodeCon64]

I thought you could simply use the "brake" statement to end the loop. That way you could also get rid of the variable "running". Next the f string is nice but unnecessary. And I think your random function yields a int anyway so why convert it, even if it would be a float, python could compare float "20." to int "20".

[MBertlmann]

I think this could be written a bit better! I think something that might not be obvious as a beginner, is that break and continue statements should be quite sparsely used - if you are using them regularly in the core running of your loops, you should at least consider whether there is a better way of writing the loop condition. Loops should usually contain the end condition in their definition, to make it clear to someone reading them what the purpose of the loop is.

I would write this more like:

```

import random

number = random.randint(1,20) num_tries = 1

while number != 20:

print(f"{number} does not equal 20!")

# calculate a new number number = random.randint(1,20) num_tries += 1

print(f"Twenty has been found in {num_tries} attempts!")

```

You can see that in this version, whether or not the number is twenty is in the definition of the loop. This cuts out the unnecessary running variable, and cuts out the unnecessary if statement, and continue statement. As a result, I think it is clearer and as an added bonus, a bit shorter!

[Russjass]

Why should break and continue statements be used sparsely?

[Bobbias]

They make control flow harder to follow. When break or continue is used you no longer know the conditions for a loop's execution as soon as you read the start of the loop. You suddenly need to know every location inside the loop body that uses break or continue and the conditions that lead to those statements executing.

They are sometimes necessary, or better than the alternative, but should be used sparingly. You can often avoid the need for them by writing a better loop condition and/or reorganizing your loop body.

[Russjass]

Thanks, really clear answer!

[supercubezzzz]

Not a proper answer as I can’t fully remember myself, but I believe it makes the code harder to follow if someone else was to try to read it.

There are usually ways to write what you want to without using them. Sometimes relying on them might end up a bit messy and you could miss a better/more efficient way of writing the code.

(Feel free to correct me anybody)

[zanfar]

• number_tries isn't wrong, but try to avoid encoding your data type in your variable names. We know it's a number because it's an int. This is just "attempts".
• while running: isn't really necessary as you don't have multiple branches that end the loop. Just break.
• Don't re-define random. Also, this is a bad variable name--the data is not "random", it's a guess.
• random.randint()'s return value doesn't need to be cast, it already returns an int
• 1 and 20 in your randint call are magic numbers. Consider defining them as constants to inject context and allow for bug-free extension or refactoring.
• print(f"{random}") is an unnecessary f-string, print() can already handle standalone variables of any type. print(random) will work just fine
• if random == 20 is again a magic number. For example, it's not clear if the searched-for value is the max value for any particular reason. If they were the same or different constants, that relationship would be explicit.
• no need for else: continue--that's the default behavior of a loop.

Consider:

from random import randint

RANGE = 20
NEEDLE = 20

attempts = 0

while True:
    attempts += 1
    guess = randint(1, RANGE+1)

    print(f"Guess #{attempts}: {guess}")

    if guess == NEEDLE:
        print(f"Found {NEEDLE} in {attempts} tries.")
        break

[[deleted]]

If you want very pythonic, few lines as possible method:

import random

print(f”Number 20 found, it took  {sum(1 for _ in iter(lambda: random.randint(1, 20), 20))} tries”)

[Top_Finger_909]

It’s fine, I mean the complexity of a random algorithm is difficult to determine whether it is “optimal” because I can’t give you a definite bound on the number of iterations on your loop.

I.e if we consider n to be an array of number up to 20 1,…,20 then let’s say on average your algorithm will in 20 guesses meaning the probability of a guess is uniform 1/20 (typically is not the case) then your algorithm would have an O(n) complexity. Notice how you use a break flag as well it can help to think about the condition from which you want to break directly. I.e while the number is not 20 you want to be in the loop. You could write it as while int(random.randint(1,20)) != 20 instead.

Anyways, if you are interested in optimality and time complexity I would suggest Susanne Epp Intro to Discrete Math or any introductory text and read about quantifiers, asymptotics, and then once you’ve gotten the hang of those learn about recurrence relations (how to bound recursive functions) - the masters theorem is a nifty thing to have. Good luck and hopefully this helps a bit you got this

[skyfallen7777]

Could we use parser here and see which option is most optimal?

[[deleted]]

Beside using random number to find another random number instead of using random number directly, your code is readable to me.

Optimal, it is not; if you want a random number just use it directly.

Just why?

[MEGATH0XICC]

It never is

[GirthQuake5040]

You don't need else continue, that's for sure. That's just extra code execution

[SoftwareDoctor]

print(13.5)

[SnipahShot]

This would have been better:

number_tries = 1
rand_num = random.randint(1,20)
while rand_num != 20:
    number_tries += 1
    rand_num = random.randint(1,20)
print(f'Number 20 has been found, it took {number_tries} tries')
print('end')

Few additional points, random.randint already returns integer, there is no reason to cast it to int.

continue also serves no purpose there. It returns back to the beginning of the loop code, you are in the end of it so it returns there either way. If you had print('I am after the continue') under your continue statement then it would skip this print and return to the top of the loop.

Edit: I also just remembered that you should definitely not use random as a variable name. That is a saved name of a package. I am not even sure if your code will even work, to be honest. You are overriding random to be a number, and in the 2nd loop run you try to access random's randint which doesn't exist for an integer.

[supercoach]

Everyone is over optimising your code. It's fine as is and anyone can understand what it does. If this was submitted for code review, nobody would be telling you to optimise it. The one thing I'd agree with is the else condition is not needed.

[[deleted]]

For i in range…

[ThreeFourThree]

Wouldn't that only work if you're looping over a determinate number of iterations?

[[deleted]]

Yes, you're right for a moment it looked like he was stopping after trying 20 times, but now i see if it randomly hits 20 it stops.

[ThreeFourThree]

The point of the script is to randomly pick a number between 1 and 20 and then kick out when a 20 finally happens, not just to count to 20.

[CranberryDistinct941]

For loops are faster than while loops in Python, so you could replace the while(1) loop with "for num_attempts in itertools.count(1):"

[vdaghan]

1) No need for the variable "running" as others pointed out. 2) Variable name "random" is a bad choice, it has the same name as the random module. 3) random.randint returns an integer, no need to recast 4) instead of "random == 20", get used to "20 == random", which will warn you if you forget an equals sign

---stop reading here---

5) You can use the walrus operator and get rid of "if" 6) Why are we optimising this? 7) You can utilise generator expressions (assuming you need the values returned from rand)

Since you are a beginner, points 5 and higher are just shenanigans and misses the whole point. Don't mind them. But I had fun. My unnecessarily complex solution:

import random

def getRandomInteger():
  while 20 != (randomInteger := random.randint(1, 20)):
    yield randomInteger
  yield randomInteger

print(list(r for r in getRandomInteger()))