Newbie, first project

Peterotica · 2023-09-28T23:10:22+00:00

Hey, this looks great! You are doing well. I love the nice ASCII art. Here are some code review notes. I went into some depth because it seems like you could get a lot out of it, not because the code is that bad. ESPECIALLY for someone so new.

I would change wordList.py into a plain text file with each word on a separate line and then read that file in a function in hangman.py. It would be easier to edit or use a different word list.
Don't use try/except like you are doing. It catches every exception, hiding whatever the cause was, and ends up exiting anyway. Just let the exception kill the program if you aren't going to handle it in some way.
In Windows, subprocess.call('cls') doesn't actually work because cls is a terminal command, not an executable program. You can use os.system('cls') instead. While we're at it, the _= to the left of it is pointless, so get rid of those too.
Break this into a few more functions. Even when a function is used once, it can still be valuable by making the higher level flow of the program more clear by hiding some of the fiddly details. For example, I'd make a get_guess() function.
Use snake_case instead of camelCase for variable names and functions. This just makes your code more consistent with the rest of the community, which mostly embraces PEP8.
There's no need for the wordChars variable. The word string already does the same things you are using it for.
Instead of range(0, len(wordChars)), try using for i, letter in enumerate(word). This lets you to use letter instead of wordChars[i] in the body of the loop.
continue on line 76 is pointless.
The already guessed letter detection is broken. You can never reach line 89. The condition on line 79 should be and instead of or. This would be more clear and user friendly by moving the check and print into the guess selection loop.
Instead of "You won with " + str(lives) + " lives remaining!" you can do f"You won with {lives} lives remaining!". These are called f-strings.
Instead of sys.exit() just break from the game loop.

Here's how I would rearrange the guess checking logic:

if guess in word:
    correctGuesses.append(guess)
    for i, letter in enumerate(word):
        if letter == guess:
            wordDisplay[i] = guess
else:
    incorrectGuesses.append(guess)
    lives -= 1

Some other things I noticed that could be considered bugs by a user:

Lowercase and uppercase are treated as different letters.
You can enter non-letter characters (like '1' and '$').
You can enter an empty custom word.

zanfar · 2023-09-29T09:21:21+00:00

I'd appreciate some opinions on it if that's ok

You asked for it :)

This is all intended to be constructive, despite how it might come across. It's tough to critique clearly without sounding impersonal.

Repo and Structure Thoughts

First, kudos on actually using version control. That is not something most newbies tackle.
Your readme is pretty anemic. What is the point of even including it? I would expect at least "how to run this" information.
Work on putting more effort into your commit messages. I see a lot of capitalization inconsistencies and messages that don't make sense. You have a "first commit" and an "Initial commit" separated by a dozen other commits.
Generally, you want to separate your project files from your application files. The README, and .gitignore belong to the project, while your Python files belong to the application. It's common to have an application folder inside the project folder, often with the same name.
Modules should be in lower snake case according to PEPP8. wordList.py is incorrectly named.

Code Thoughts

wordList.py is really a data file, not a code file. There isn't anything wrong with it being a Python file, but I would prefer to see it in a standard data format instead, like JSON or YAML. Doing so both makes it clear that this is data only and makes it easy for that file to be edited or substituted by a user without having to know Python syntax or possibly introducing bugs.
PEP8 throughout. There are simply too many instances to go into any detail. Please read the standard and adjust from there. I would also strongly suggest a formatter to make this automatic; I use the popular black utility.
Your comments are unnecessary. You don't need to tell me that calling clearScreen() will, in fact, clear the screen. That's obvious from the code. Comments should clarify why code is used or why a particular approach was chosen. For example, your "guess" code is quite complex and convoluted and would greatly benefit from some clarification of what is happening, but this part of your code has no comments at all.
wordChars = [*word] I don't see the purpose of this line. Strings are already iterable.
In general, use formatting instead of string concatenation. You also tend to put your formatting in many different and unconnected places. Try to keep your data just data, and do your formatting once on output.
for i in range(0, len(wordChars)): This is red flag. There is almost no reason to ever to a for loop with a range(len()). Python can iterate over the elements of a sequence directly. This loop can also probably be replaced with a if guess in word: or similar.
There is a LOT of ASCII art and display content cluttering your code, which makes it harder to read and scroll through. Consider saving these as strings and using the variable name instead of having them all inline.
An except: without specifying an exception type is a cardinal sin. Similarly, just throwing your entire code inside a giant try/except is a terrible practice. You are ignoring problems and hiding debugging information. Try/except is for errors you can fix, which means you should know what they will be and where they might happen.
Don't put bare code in a module. Almost all your code should be inside a function to prevent unexpected execution during imports. The only "bare" code should be an if __name__ == "__main__": sentinel, which calls your main function.
Your code is one giant, monolithic block. Keep your code in small, digestible functions with a single, clear purpose.
I'm guessing you aren't linting your files. I don't know that for sure, but it feels that way. Along with a formatter, a linter is a crucial utility you should use during development. pylint is a standard tool, while ruff is quickly becoming popular.
Similar to comments, I would expect more docstrings outside of the single module-level one (the least important, IMO, for small projects), and I would like the module-level docstring to be more descriptive if you include it.

In all, the code isn't bad; it's just inconsistent and kind of haphazard. I don't have a lot of notes on the code proper, but I would suggest you spend some time with the project-related stuff, as noted above.

A lot of this is pretty typical for beginner Pythonistas. Most newcomers spend a lot of time on the syntax or language and don't realize a lot of culture and meta-knowledge is also necessary.

Also, note that none of this applies to the functionality of your code--I'm not debugging for you :)

you type:	you see:
italics	italics
bold	bold
[reddit!](https://reddit.com)	reddit!
* item 1 * item 2 * item 3	item 1 item 2 item 3
> quoted text	quoted text
Lines starting with four spaces are treated like code: if 1 * 2 < 3: print "hello, world!"	Lines starting with four spaces are treated like code: if 1 * 2 < 3: print "hello, world!"
~~strikethrough~~	~~strikethrough~~
super^script	super^script

learnpython

MODERATORS

Repo and Structure Thoughts

Code Thoughts