Skip to content
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

Sharks - Sujin and Danielle #49

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
113 changes: 109 additions & 4 deletions adagrams/game.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,116 @@
import random


LETTER_POOL = {
'A': 9,
'B': 2,
'C': 2,
'D': 4,
'E': 12,
'F': 2,
'G': 3,
'H': 2,
'I': 9,
'J': 1,
'K': 1,
'L': 4,
'M': 2,
'N': 6,
'O': 8,
'P': 2,
'Q': 1,
'R': 6,
'S': 4,
'T': 6,
'U': 4,
'V': 2,
'W': 2,
'X': 1,
'Y': 2,
'Z': 1
}

LETTER_SCORE = {
'A': 1,
'B': 3,
'C': 3,
'D': 2,
'E': 1,
'F': 4,
'G': 2,
'H': 4,
'I': 1,
'J': 8,
'K': 5,
'L': 1,
'M': 3,
'N': 1,
'O': 1,
'P': 3,
'Q': 10,
'R': 1,
'S': 1,
'T': 1,
'U': 1,
'V': 4,
'W': 4,
'X': 8,
'Y': 4,
'Z': 10
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like that these are defined as "constants" outside the function. Moving large chunks of data out from a function helps declutter the function.


def draw_letters():
pass

user_letters = []
copy_letter_pool = dict(LETTER_POOL)
print(copy_letter_pool)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can remove debugging print statements before submitting your project to keep your functions clean.

while len(user_letters) < 10:
random_letter = random.choice(list(copy_letter_pool.keys()))

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Calling list() on a dictionary takes all the keys and puts them in a list so we get a list of 26 letters here.

In order to select a random letter from a pool that reflects what you have declared in LETTER_POOL, you could create a list pool_of_letters and use the keys and values from LETTER_POOL to populate a list with 9 As, 2 Bs, 2 C,s etc.

pool_of_letters = []
for letter, number in LETTER_POOL.items():
        pool_of_letters += letter * number

# You could then do user `random.choice(pool_of_letters)` 

This is just one approach to populating pool_of_letters, feel free to use any other approach that comes to mind too.

user_letters.append(random_letter)
copy_letter_pool.update({random_letter:copy_letter_pool[random_letter] - 1}) # Reduce value of letter by 1
if copy_letter_pool[random_letter] == 0: # Delete any letter from dict with value of 0
del copy_letter_pool[random_letter]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can get around having to delete from key-value deleting by adding a check a little earlier in this function.

Adding a check after line 68 to check if the value of copy_letter_pool is greater than 0 before appending random_letter to user_letters makes it so you wouldn't need to delete anything from the dictionary.


return user_letters


def uses_available_letters(word, letter_bank):
pass
aCopy = list(letter_bank)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function names should be lowercase, with words separated by underscores to adhere to the Python style guide: readability.https://peps.python.org/pep-0008/#function-and-variable-names

for letter in word:
if letter.upper() in aCopy:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The in operator on lists has linear time complexity on top of the first loop that's running on loop 79. You can optimize this here by using letter_bank to create a frequency dictionary where the keys are the letters and the values are the frequency of the letter. For example: letter_frequency = {"a": 1, "e": 2}

Then when you check if a letter is a key of letter_frequency, you reduce your time complexity to O(1).

for letter in word:
     if letter in letter_frequency:

Take care to use key in dict syntax. If you do key in dict.keys() you'd reproduce O(n) time complexity since under the hood Python creates a list of keys when you call .keys() and the in operator for lists is linear.

aCopy.remove(letter.upper())
else:
return False
return True


def score_word(word):

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

pass
score = 0
for letter in word.upper():
score += LETTER_SCORE[letter]
if len(word) >= 7:
score += 8

return score

def get_highest_word_score(word_list):
pass
highestScore = 0
winningWord = ""
Comment on lines +97 to +98

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Python uses snake_case for naming variables so these could be renamed to highest_score and winning_word.


for word in word_list:
if score_word(word) > highestScore:
highestScore = score_word(word)
winningWord = word

elif score_word(word) == highestScore:
if len(word) == 10 and len(winningWord) != 10:
winningWord = word
if len(word) < len(winningWord) and len(winningWord) == 10:
winningWord != word
elif len(word) < len(winningWord):
winningWord = word
Comment on lines +105 to +111

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could also pull this into a helper function to help simplify the logic in this function overall.


return (winningWord, highestScore)