I started learning Python and wrote my first program. This is a console game based on guessing the number. Can you take a look and tell me if I'm doing it right or give me any advice? Here is the code: https://github.com/theriseq/GuessTheNumber
Is it a good way of doing this or can I simplify it?
Thank you.
main.py
from can_convert import check
import random
welcome_msg = """Welcome to GuessTheNumber Game!
I\'m thinking of a number between 1 and 100.
Can you guess this number?
[?] Type \'pass\' to exit game.
"""
print(welcome_msg)
number_to_guess = random.randrange(1, 101)
attemps = 0
while True:
user_inp = str(input("Your choice: "))
if user_inp != "pass":
if check(user_inp):
attemps += 1
# print('is int')
if int(user_inp) < number_to_guess:
print("Your guess is too small.")
elif int(user_inp) > number_to_guess:
print("Your guess is too high.")
else:
print("Correct! You won the game!")
print(f"Attemps: {attemps}")
break
else:
print('Enter numbers only!')
else:
break
can_convert.py
def check(to_check):
try:
int(to_check)
return True
except ValueError:
return False
-
3\$\begingroup\$ Please edit your title to be on-topic, aka claim what the code does, as opposed to the currently very generic title. \$\endgroup\$Alex Povel– Alex Povel2020年05月01日 11:46:19 +00:00Commented May 1, 2020 at 11:46
-
1\$\begingroup\$ Welcome to CR SE! Alex Povel is right that you need to rename the question, but the question body looks ok, and I'll try to post a response if work's not too busy this morning! \$\endgroup\$ShapeOfMatter– ShapeOfMatter2020年05月01日 12:03:22 +00:00Commented May 1, 2020 at 12:03
-
\$\begingroup\$ Technically we need the challenge description to be inside the question body, but a number-guessing-game is as common as FizzBuzz nowadays. Please note this for future questions though, good luck. \$\endgroup\$Mast– Mast ♦2020年05月01日 18:50:38 +00:00Commented May 1, 2020 at 18:50
-
\$\begingroup\$ @AlexPovel Titles don't make questions on or off-topic. A bad title gets a downvote, but not a close vote. \$\endgroup\$Peilonrayz– Peilonrayz ♦2020年05月03日 02:35:07 +00:00Commented May 3, 2020 at 2:35
2 Answers 2
Here is a list of found issues, in order of occurrence:
- importing from your custom
can_convert
is fine. You should stick to modularization like that for bigger projects. However, here, it actually seems to be in the way of things. It checks if the string can be converted, but does not actually do any conversion. We later have to do the conversion again anyway. This can be combined. - Currently, the welcome message and actual
random
range can deviate. You already know whatf
-strings are, so they should be used to keep that in line. - Use
randint
to include both bounds, as opposed torandrange
, which would stop one short ofupper_bound
, as you already noted. input
will already return a string, no need to convert viastr
.- By inverting the logic in checking for
"pass"
, a whole block of indentation can be spared. Just check ifuser_inp == "pass"
andbreak
ifTrue
. All following code can be one level less indented, since noelse
statement is required. Now for the main logic. The
try
block has moved here, away from the import. Notice that the logic is still the same. However, we assign tonumber
immediately without checking first. If this fails, theexcept
block is triggered.- Note that
int("2.2")
will also be aValueError
despite"2.2"
, to humans, clearly being a number. So the error message should specifywhole
numbers to be more precise. - The added
else
block is run if noexception
was raised, so in the normal game. number
is now available, so no need to callint
conversion repeatedly.
Keep
try
blocks short and concise and never leave anexcept
statement bare. You did this fine! Lastly, try to not nesttry/except
blocks.- Note that
- There was a typo for
attempts
.
import random
upper_bound = 100
lower_bound = 1
welcome_msg = f"""Welcome to GuessTheNumber Game!
I'm thinking of a number between {lower_bound} and {upper_bound}.
Can you guess this number?
[?] Type 'pass' to exit game.
"""
print(welcome_msg)
number_to_guess = random.randint(lower_bound, upper_bound)
attempts = 0
while True:
user_inp = input("Your choice: ")
if user_inp == "pass":
break
try:
number = int(user_inp)
except ValueError:
print('Enter whole numbers only!')
else: # no exception occurred
attempts += 1
if number < number_to_guess:
print("Your guess is too small.")
elif number > number_to_guess:
print("Your guess is too high.")
else:
print("Correct! You won the game!")
print(f"Attempts: {attempts}")
break
You've done a great job demoing a variety of patterns and tools. I'll add two more things you could benefit from learning early:
- Type hints. Function headers are almost always clearer with type hints, and you need them if you want to use a tool like MyPy.
- The main method pattern.
- (Usually I'd also advocate for recursion when someone's demo project is a REPL game like this, but in your case I don't think it would improve this code.)
Other stuff:
- Usually it's better to avoid exceptions when you have other options. While you could do what you're doing here by other means, you've done a few things that make me like your
check
function: It's concise, it bundles away a discrete bit of functionality, and you're catching a narrowly defined exception class. - Unfortunately, it's a little clunky to have a function that just tells you if
int(x)
threw an exception, just so you can callint(x)
on the next line. You have many other options; I'd be fine withdef check (user_input: str) -> Optional[int]:...
, but then you must useif x is [not] None:...
later. - Depending exactly what the function in question does, either
validate
orsanitize
would probably be better thancheck
. - I lied: another new thing to learn: itertools. In particular, a
while
loop that increments something is always begging to get replaced with afor
loop. In this case, since we want to keep going "as long as it takes", we need a lazy infinite iteratable:itertools.count(0)
.- In order to make that work, we'll need to separate out the "that input was invalid, try again" logic into a separate loop (or recursive function).
- And then if you teach yourself generators you could write
for (try_number, user_input) in zip(itertools.count(0), yielded_user_input()):...
. Fun times!
-
\$\begingroup\$ "Usually it's better to avoid exceptions when you have other options.", do you have an example? One replacement for exceptions can lead to an abundance of
isinstance
,hasattr
,issubclass
or similar checks (Look Before You Leap), which is mostly more verbose with little added benefit. See also here. \$\endgroup\$Alex Povel– Alex Povel2020年05月01日 14:41:59 +00:00Commented May 1, 2020 at 14:41 -
1\$\begingroup\$ @AlexPovel Where EAFP may be Python-idiomatic, (1) there are exceptions and (2) it's not my personal style. Consider the difference between
try: dict[k] except KeyError
vs.dict.get[k]
, where I would prefer the latter. \$\endgroup\$Reinderien– Reinderien2020年05月01日 15:06:37 +00:00Commented May 1, 2020 at 15:06 -
1\$\begingroup\$ People throw exceptions and catch exceptions all the time and it's fine. But I do claim that you should always consider other options. If what you're trying to say is
if is_valid(x): y(x) else: z()
then you wouldn't want to writetry: y(x) except Barr: z()
. Also, while it seems trite to say "There are a lot of ways you can do exceptions badly", people really do make mistakes, and other patterns really do have fewer pitfalls. \$\endgroup\$ShapeOfMatter– ShapeOfMatter2020年05月01日 16:52:54 +00:00Commented May 1, 2020 at 16:52 -
\$\begingroup\$ Okay, both these are good points. \$\endgroup\$Alex Povel– Alex Povel2020年05月01日 17:44:19 +00:00Commented May 1, 2020 at 17:44
-
1\$\begingroup\$ "The main method pattern" links to MyPy, Is that supposed to be a different link? \$\endgroup\$lights0123– lights01232020年05月01日 18:24:02 +00:00Commented May 1, 2020 at 18:24
Explore related questions
See similar questions with these tags.