-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Hangman assignment #1
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great coding overall.. looks like you're good to go for OOP!
|
||
# Complete with your own, just for fun :) | ||
LIST_OF_WORDS = [] | ||
LIST_OF_WORDS = ['cookies', 'cream', 'love', 'vinegar'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great words !
if list_of_words == []: | ||
raise InvalidListOfWordsException | ||
else: | ||
return random.choice(list_of_words) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
very clean code here ... random library is perfect for this...
if word == "": | ||
raise InvalidWordException | ||
else: | ||
return len(word)*"*" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
another piece of clean code!
raise InvalidGuessedLetterException(Exception) | ||
if len(masked_word) != len(answer_word): | ||
raise InvalidWordException | ||
if answer_word =="" or masked_word =="" : |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's possible to combine all conditions in line 41 with "or" for the same InvalidWordException
#convert answer_word and character to lower case | ||
lower_character = str.lower(character) | ||
lower_answer = str.lower(answer_word) | ||
if lower_character not in lower_answer: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can remove line 48 and 49 since there's no change to masked_word ... notice line 55 will return masked_word anyway ...
if lower_character in lower_answer: | ||
indices=find_substring(lower_answer, lower_character) | ||
for index in indices: | ||
masked_word = masked_word[:index] + lower_character + masked_word[index + 1:] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great restructuring of word there!
raise GameFinishedException | ||
else: | ||
#make game case insensitive by converting all strings to lower case | ||
letter = str.lower(letter) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good idea to lowercase the letter and answer_word here.. which means you can remove the lowercase at _uncover_word() function ...!
For code review