I have been trying to write a number guessing game and I am trying to condense the second while loop when guess =! random_number.
I can't figure out if there is any better way to shorten the code but to still achieve the same result. I wonder if someone is able to help me out.
If there's anything else which is redundant or not efficient in my code or how i could code it better please criticize the hell out me my work!
import random
def inputNumber(message):
while True:
try:
userInput = int(input(message))
except ValueError:
print("Not an integer! Try again.")
continue
else:
return userInput
break
#ranges must be integers
min_range = int(inputNumber("Please insert the min range: "))
temp_range = int(inputNumber("Please insert the max range: "))
#max range must be bigger than min
while min_range >= temp_range:
temp_range = int(inputNumber("Please insert a max range which is smaller than min range!: ")) + 1
max_range = temp_range
random_number = random.randint(min_range, max_range)
guess = int(input("Please enter your guess: "))
counter = 1; # you guessed once already
while guess != random_number: # 1 guess incorrect
print(f"{random_number} is the random number")# to see for myself which is the random numb
if min_range < guess < max_range: # WITHIN THE RANGE
if random_number - random_number / 12 < guess-random_number < random_number + random_number / 12: # 3 super hot
print("Please try again, you are super hot!")
elif random_number - random_number / 8 < guess-random_number < random_number + random_number / 8: # 3 hot
print("Please try again, you are hot!")
elif guess < random_number - random_number / 2 or guess-random_number > random_number + random_number / 2: # 3 cold
print("Please try again, you are cold!")
else: # OUTSIDE RANGE
print("You are outside the range, try again!")
guess = int(input())
counter += 1
if guess == random_number: # guess correct
print(f"Congratulations, you guessed correctly in {counter} tries!")
edit: thanks you all so much for taking the time to review my code, i will try to improve on the noted points, much appreciated (:
-
2\$\begingroup\$ Welcome to Code Review! Please edit your question so that the title describes the purpose of the code, rather than its mechanism. We really need to understand the motivational context to give good reviews. Thanks! \$\endgroup\$Toby Speight– Toby Speight2021年05月20日 18:39:54 +00:00Commented May 20, 2021 at 18:39
3 Answers 3
Miscellaneous notes:
- you don't need a
break
after areturn
- you could (attempt to) return directly from the input; "except" will still catch and cycle errors
inputNumber()
explicitly returns an integer; no need to cast again.temp_range
isn't really needed - you can just usemax_range
, no loss to readability.- no need to add one to
max_range
(in the validation loop) -randint()
includes the top limit. random_number
is an unhelpful variable name - I've called thistarget
- using
inputNumber()
for theguess
also seems sensible. guess
should be checked against the inclusive range - min and max are possible values.- personally I think the temperature categories should reflect the size of the range, rather than the size of the chosen number... however
- your calculation is wrong anyway; you are testing the difference between
guess
andtarget
so you should test against the small fraction oftarget
, nottarget
reduced by a fraction (or should just checkguess
against the numbers you calculate). - the "hot" range (outside "super hot") is actually smaller than the "super hot" range - I adjusted this out to 1/4 of
target
- after correction, there's a range that gets no message, which can be covered in an
else
clause. I reordered so all tests are for "in this range" and theelse
covers the "cold" option.
To condense the message selection, you could set up a list of boundaries changing from one condition to another, and step through (or search) to find the applicable message. However I'm not sure that's a lot clearer in this case.
With updates reflecting most of these:
import random
CHEATING = True
def inputNumber(message):
while True:
try:
return int(input(message))
except ValueError:
print("Not an integer! Try again.")
#ranges must be integers
min_range = inputNumber("Please insert the min range: ")
max_range = inputNumber("Please insert the max range: ")
#max range must be bigger than min
while min_range >= max_range:
max_range = inputNumber("Please insert a max range which is bigger than min range!: ")
target = random.randint(min_range, max_range)
guess = inputNumber("Please enter your guess: ")
counter = 1; # you guessed once already
while guess != target: # 1 guess incorrect
if CHEATING: print(f"{target} is the random number")# to see for myself which is the random numb
if min_range <= guess <= max_range: # WITHIN THE RANGE
if -target / 12 < guess-target < target / 12: # 3 super hot
print("Please try again, you are super hot!")
elif -target / 4 < guess-target < target / 4: # 3 hot
print("Please try again, you are hot!")
elif -target / 2 < guess-target < target / 2: # a bit meh
print("Not great, not terrible - please try again")
else: # 3 cold
print("Please try again, you are cold!")
else: # OUTSIDE RANGE
print("You are outside the range, try again!")
guess = inputNumber("Next guess? ")
counter += 1
if guess == target: # guess correct
print(f"Congratulations, you guessed correctly in {counter} tries!")```
PS: "please criticize the hell out my work" is an excellent attitude, if you have the fortitude for it, and a great way to learn.
The biggest sources of repetition in your code are the mathematical
calculations of the difference between the random number and the user's guess,
along with the need to re-ask the user for another guess on a couple of
occasions. You already have a function to get an integer from the user, so take
advantage of it! Also, you could enhance that function to eliminate the need to
repeat yourself when collecting max_range
. Finally, I did not
fully understand the intended logic of your difference calculation,
so I improvised to use a modified approach
(percentage difference relative to the overall span of the range). In any case,
the important point is to make the calculation once. Then use the result to
select the adjective. Some possible edits for you to consider:
import random
# An enhanced function taking an optional minimum value.
# If needed, you could enhance further by changing that
# parameter to a `range()` if you want to restrict both sides.
# If you were do to that, you could further simplify the
# guessing loop.
def inputNumber(message, min_val = 0):
while True:
try:
n = int(input(message))
if n >= min_val:
return n
else:
print(f"Must be greater than {min_val}. Try again")
except ValueError:
print("Not an integer! Try again.")
# No need to convert these values to integers.
# The function already does that.
min_range = inputNumber("Please insert the min range: ")
max_range = inputNumber("Please insert the max range: ", min_val = min_range)
random_number = random.randint(min_range, max_range)
counter = 0
# When getting user input, a while-True loop is usually
# the right way to start. If you ask before the loop,
# you have to re-ask if the user makes a mistake.
while True:
counter += 1
guess = inputNumber("Please enter your guess: ")
if guess == random_number:
print(f"Congratulations, you guessed correctly in {counter} tries!")
break
# This comparison should use <= rather than < I think.
elif min_range <= guess <= max_range:
diff_pct = abs(guess - random_number) / (max_range - min_range)
adjective = (
'super hot' if diff_pct < .2 else
'hot' if diff_pct < .4 else
'cold'
)
print(f"Please try again, you are {adjective}!")
else:
print("You are outside the range, try again!")
I'll add some points to Joffan's answer, which already provides some great improvements.
Naming
PEP 8: Variable and function names should follow snake_case instead of camelCase.
input_number
This function should be a lot simpler:
def input_number(message):
while True:
try:
return int(input(message))
except ValueError:
print("Not an integer! Try again.")
main()
It's a good idea to wrap your procedure into a main()
function, which provides a standard entry point. It also makes the functionality of your code clearer at a glance. Since a function will not run on its own when we execute the script, we need to call it. For most use cases, we should wrap it in a if __name__ == "__main__":
condition.
def main():
# this is where your procedure now runs
if __name__ == "__main__":
main()