Skip to content

Phoenix - Dana Delgado #43

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

Open
wants to merge 2 commits into
base: main
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
128 changes: 123 additions & 5 deletions adagrams/game.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,129 @@
def draw_letters():
pass
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
}
def draw_letters():

# Create an empty list to hold the letters from LETTER_POOL
letter_list = []

# Go through each letter and its quantities in the letter pool
for letter, quantity in LETTER_POOL.items():

Choose a reason for hiding this comment

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

Great job with taking the distribution of letters into account! Great use of the extend method! This is short, sweet and concise and works really well!

# Add the letter to the list, repeating according to its quantity
letter_list.extend([letter]*quantity)

# Create another empty list for the letter that will be drawn
hand_of_ten_letters = []

# Choose 10 letters
for _ in range(10):
# Select a random index in the list
index = random.randint(0, len(letter_list)-1)
# Add the letter at the index to the list of drawn letters
hand_of_ten_letters.append(letter_list[index])
# Remove that letter from the list so we don't choose it again
letter_list.pop(index)

# Return the 10 drawn letters
return hand_of_ten_letters

Choose a reason for hiding this comment

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

Great job on this function! Everything looks great, and it works really well! The one thing to note is that you include a comment before pretty much every line of code. While this is great for understanding your code, it can start to busy things up. I would recommend either placing the comments in a docstring at the beginning of the function as a quick write up of how the function/ algorithm works or as comments to kick off chunks of code rather than before each line!


def uses_available_letters(word, letter_bank):
pass

# Covert all letters to uppercase
word = word.upper()
letter_bank = [letter.upper() for letter in letter_bank]

Choose a reason for hiding this comment

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

Great list comprehension!


# Create an empty dictionary to count the letters in the letter bank
available_letters = {}
for letter in letter_bank:

Choose a reason for hiding this comment

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

One small thing to think about is that you are creating a dictionary holding all the available letters along with their count. You are creating that dictionary from the letter bank list that you so beautifully created using a list comprehension. My thought here though is that you are not destroying the list you've created in any way, just using the values. Therefore, the list comprehension on line 60 becomes redundant and adds a for loop that doesn't need to exist. We could simply run the letter.upper() on each letter in this for loop and add it to the dictionary rather than include a whole new list comprehension! If we were instead using that comprehension to copy over the letter_bank and then destroying it later on, I could see a use, but for now it just adds an extra for loop!

if letter in available_letters:
available_letters[letter] += 1
else:
available_letters[letter] = 1

# Check if every letter in the word is available in the letter bank and has the required quantitiy
for letter in word:
if letter in available_letters and available_letters[letter] > 0:
available_letters[letter] -= 1
else:
return False

Choose a reason for hiding this comment

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

Given what you've got, this for loop looks great!


return True


def score_word(word):
pass

# Create a dictionary based on the score chart
letter_values = {

Choose a reason for hiding this comment

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

When we've got a hard coded dictionary like this, its usually better served as a global variable! Especially if that dictionary was needed for other functions in an expanded version of this project!

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

# Covert the word to upper case
word = word.upper()
total_score = 0

# Calcualte the base score
for letter in word:
if letter in letter_values:
total_score += letter_values[letter]

# Add bonus pointsif the world lenght is equal or greater to 7
if len(word) >=7:

Choose a reason for hiding this comment

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

Small thing, but we have one other constraint on the length of the word! It can't go over 10 letters! If you want to, see if you can add that constraint to this condition!

total_score += 8

return total_score

def get_highest_word_score(word_list):
pass
highest_word = ""
highest_score = 0

for word in word_list:
# Get the score of the current word
score = score_word(word)

# If this score is higher than what we've got

Choose a reason for hiding this comment

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

This function and all the logic that goes along with it looks great! I love the way you only focused on words that were of equal or greater value and anything else was ignored! As mentioned before, the one change I would make is to watch your comments! While helpful in the early stages of programming, comments before every line aren't the norm in most work places, so feel free to get in the habit of trying to limit your comments to code blocks or a docstring!

if score > highest_score:
#Update the best word
highest_word = word
# Update the best score
highest_score = score
elif score == highest_score:
# Prefer the ten letter word if available
if len(word) == 10 and len(highest_word) != 10:
highest_word = word
# Prefer the shorter word if they are the same score
elif len(word) < len(highest_word) and len(highest_word) <10:
highest_word = word
# If both are the same lenght, choose the one that shows first in the list
elif len(word) == len(highest_word):

Choose a reason for hiding this comment

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

The only thing this condition does is continue on to the next iteration of the for loop. Since this is the last condition that would be checked before the loop moves to its next iteration, it actually isn't needed! It should work the same way with or without it!

# Do nothing and keep the current highest word
continue

return (highest_word, highest_score)

Choose a reason for hiding this comment

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

Small not, but when we're returning a tuple like this, we actually don't need the parentheses! You are more than welcome to put them in if you'd like but the code will work the same way even if they aren't present!