As it is, your program is already in a pretty good shape. There is an error in the logic of the input part (what happens if the user enters a string longer than one character? what happens if they enter an empty string?), but apart from that, it's a good starting point.
But there are several improvements that you can do in order to make your code more idiomatic, to improve the readability, and to give it a more concise structure. Your program logic basically consists of three parts: an initialization, an input part, and an evaluation. You have a separate function for the last part (your function checkifcorrect()
), but not for the first two parts. This is the first change that I'd recommend, and my answer is basically progressing through these three parts.
Initialization
Your initialization is fairly simple. All that needs to be done is that you need a list of five randomly chosen letters. You decided to use the choice()
function for that, which does work. But there's an even more tailor-suited function for your use case: choices(lst, k)
, which creates a list of k
choices from lst
(with replacement, i.e. the same elements can be chosen more than once). Your initialization function will now contain of a single line of code:
def assign_letters():
return choices(ascii_uppercase, k=5)
User input
There is a Python idiom that you will frequently encounter when a program requires a valid input from the user (but also in many other places). This idiom uses a while True:
loop. In your case, it could look like this:
while True:
user_input = input("Enter a letter: ")
if len(user_input) == 1 and user_input.isalpha():
break
The logic here basically reverses the logic of your current while
loop. Currently, the loop is repeated for as long as the previous input is invalid. The while True:
loop is repeated forever – unless it's explicitly left by the break
instruction if the current input is valid. This results in a very strict regimen of what is considered a correct input, and it has the advantage that you don't need two more or less identical input()
instructions.
When it comes to creating the input prompts, you may want to look into Python format strings. This is a subtype of strings that comes in handy whenever you want to create a string that embeds the value of a variable (or for that matter, the value of any evaluation). You create a format string are like any other strings, but you prepend the opening quotation mark by the letter f
. Furthermore, at the point where you want your variable to appear, you insert the name of your variable enclosed in curly brackets. In your case, you can use the following line for input:
user_input = input(f"User {i+1} enter a character from A-Z:\n")
At runtime, the sequence "{i+1}"
will automatically be replaced by the current value of i+1
.
Evaluation
You already have a function checkifcorrect
that evaluates the answers. It does its job, but there are a few improvements here as well. The first improvement concerns the name checkifcorrect
. There is the Python Style Guide "PEP 8" that describes the style of well-formed Python scripts, including naming conventions. While these conventions are only recommendations, they are are chosen to improve the readability of Python scripts, and they are in very common use. So in order to help others read your scripts, it's always a good idea to stick to PEP8. For function names, the recommendation is to use lowercase words combined with underscore characters. So, PEP8 would recommend check_if_correct()
as a name.
This also means that the variable name randompc
is not really well-formed – I will use random_pc
from now on. Speaking of which: the next improvement concerns variable handling. As it is, your function check_if_correct()
accesses the global variables user_answers
and random_pc
. These variables are called "global" because they were not created within the function check_if_correct()
, and they were not passed as arguments to the function call. This has a downside: your function will fail to work if these variables are not created beforehand.
In a small problem like your assignment, this won't matter much, but even in slightly bigger scripts you don't want your functions to rely on variables that are created elsewhere. Very generally speaking, global variables are useful, but they need to be used with care, and more often than not, you want to rely on local variables instead (this is something that beginners often need to get used to). So, instead of using the global variables user_answers
and random_pc
, you should add these variable names as arguments to the function definition:
def check_if_correct(user_answers, random_pc):
With this definition, you can pass any variable to your function regardless of the name of that variable in the global scope.
The next thing to improve would be to replace the + str(i+1)
bits and use format strings again when you print the user numbers:
if user_answers[i] == random_pc[i]:
print(f"Good Job user {i+1}")
else:
print(f"Good luck next time user {i+1}")
The last improvement concerns printing the list random_pc
. Your line print(*random_pc, sep=", ")
is actually a pretty clever solution – using *
to pass the elements of a list as arguments to a function is a very useful trick to know.
But if you want to combine the elements of a list into a single string, there's a more idiomatic way that works independently of the print function with the sep
argument. Every string provides the method join(lst)
. This method returns a new string that uses the original string as a separator between each element of the list lst
. So, in your case, if you want to print the elements of random_pc
separated by ", "
, you can use the following command:
print(", ".join(random_pc))
Granted, it's not really shorter or less complex than your command, but it's the more conventional way if list elements are to be joined in a string.
Overall structure
Now, you have three functions: assign_letters()
which returns a list containing the randomly chosen letters per player, get_guesses()
which returns a list containing the guesses of each player, and check_if_correct()
that compares each guess to the randomly chosen letter. We can combine them into a single code block:
letters = assign_letters()
guesses = get_guesses()
check_if_correct(guesses, letters)
This code clearly reflects the program logic: first, letters are assigned, then, guesses are received, and then, the guesses and letters are checked for correctness.
With regard to the placement of this code block, there's another Python idiom that you'll find in virtually every non-trivial script: at the end of the main script, you will find the code block that contains every line of code that should be executed if the script is started. And this code block is wrapped into a somewhat cryptic if
condition, like so:
if __name__ == "__main__":
letters = assign_letters()
guesses = get_guesses()
check_if_correct(guesses, letters)
This works because unless the script was imported as a module, the internal variable __name__
will contain the string "__main__"
as its value. Technically speaking, this is the way to identify the "top-level script environment", and there's a Stackoverflow question that will explain it in detail.
All in one place
So, here's the sum of all of my recommendations:
from string import ascii_uppercase
from random import choices
def assign_letters():
return choices(ascii_uppercase, k=5)
def get_guesses():
user_answers = []
for i in range(5):
while True:
user_input = input(f"User {i+1} enter a character from A-Z:\n")
if len(user_input) == 1 and user_input.isalpha():
break
user_answers.append(user_input.upper())
return user_answers
def check_if_correct(user_answers, random_pc):
for i in range(len(user_answers)):
if user_answers[i] == random_pc[i]:
print(f"Good Job user {i+1}")
else:
print(f"Good luck next time user {i+1}")
print("The answers were: ")
print(", ".join(random_pc))
if __name__ == "__main__":
letters = assign_letters()
guesses = get_guesses()
check_if_correct(guesses, letters)
And I do agree with @zachary-vance that your teacher assigned you an incredibly stupid, un-fun game to code.