I apologize that this answer became a "stream of conscious"-type answer. I found it difficult to properly categorize what I wanted to say.
There's a fair amount to be said in add_players. The way you have things set up is quite confusing. You have fairly deep nesting, and multiple returns at multiple different levels of nesting.
if pla not in (a for i in players for a in i) is also confusing. Think about for i in players. You're referring to each player as i? Your intent with that generator expression also seems to be to flatten players so you can check if a name already exists. That doesn't seem necessary though since each "player" is a name and score. Why check if a name is equal to a score?
You also have a while len(players) < 4 inside of a while True. I can't really see the point of the while True though.
I would also use much more descriptive names. Once you reduce the nesting, you'll have a lot more room to work with.
Finally, add_players really shouldn't be adding to a global players. players should be returned from add_players and assigned at the call site.
Altogether, I'd write this closer too:
def add_players():
players = []
while len(players) < 4:
new_name = input("Enter Player Names (can have up to 4) >>> ")
if new_name:
if new_name not in (name for name, _ in players):
players.append([new_name, 0])
print("Player {} added".format(new_name))
else:
print("Name already in players' list")
else:
break
return players
Notice now There's a single return, and it's returning the players for the caller to use.
Please don't take this the wrong way, be the design of the rest of the program doesn't make much sense:
Why is everything inside of main? Ideally, the main function should be a small function at the end of your program that just calls a few other functions.
Why is a Score class inside of the home function?
Score also doesn't seem like it should even be a class. It seems like you're only using Score to score a word by calling the Score constructor, and all of the methods of the class are static. If you never use an instance of a class, it shouldn't be a class. Just make Score into a score_word function. I'd also pass a word into the function instead of asking inside of the function. Functions are harder to test when they produce their own data.
You're catching a NameError in undo. I'm guessing this is in case Score() hadn't been called yet? Don't do this. I can't think of a time when it's ever appropriate to catch a NameError. Any time a NameError happens, it means you have flawed logic in your program. You should fix the issue instead of putting an try/except band-aid over it. If the function requires temp_p and temp_v (which need better names), you need to ensure that that data is available. Either pass it in, or ensure that they're properly initialized ahead of time.
There's more than can be dug out here, but I need to start getting ready for work. I'll just make a few broad suggestions:
- Please take much more care when writing code. A lot of this code seems like it was written quickly without much second-thought. I would focus on the intent of the code before it's written.
Ask yourself, "What exactly should this code do?", and "What are the appropriate tools to do what I'm trying to do?". Doing things like using classes as normal functions and writing code like (a for i in players for a in i) that technically works (but doesn't really do exactly what you want) makes your code hard to understand, and will make it difficult to add to later.
- Don't nest everything. Really, all of these functions should be "top-level", outside of everything else. If you nest thing
A inside of thing B, that suggests that A is deeply connected to B and wouldn't have a valid meaning outside of B. Really though, that's not the case here. Score would have the same meaning even if it were outside of home, and all the functions would have the same meaning even if they were outside of main.
Nesting conveys a certain association meaning, and if used improperly, gives the reader the wrong initial idea about how the code works. Nesting also forces you to use more indentation, which generally makes code harder to read.
With the above criticisms out of the way, I'll note what is good here:
You're following proper naming guidelines. You have functions starting with lowercase, classes starting with uppercase, and _ to separate "words" in names.
You're using idiomatic shortcuts like if pla: to check if a collection is empty or not.
You tried to have all the functionality start from a main instead of being loose in the script. This makes code easier to run and test.
letter_val is a good use of a dictionary.
I'll note though that it's a constant, so it should be uppercase.
Keep at it. There's a lot to be improved here, but there's promise too.
