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.

Sign up for GitHub

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

Jump to bottom

Sapphire- Janice N. #115

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Sapphire- Janice N. #115

Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
24532de
added comment to function
janice15 Mar 16, 2023
e9a6ba1
passed three tests, failed 1
janice15 Mar 19, 2023
4d42889
passed 4 tests
janice15 Mar 21, 2023
96f110e
passed all wave 3 tests
janice15 Mar 22, 2023
d1bbdf5
passed 5/10 tests in wave 4
janice15 Mar 23, 2023
e46c2d8
passed all tests for wave 2
janice15 Mar 23, 2023
18f68a5
added comments to code
janice15 Mar 24, 2023
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
127 changes: 122 additions & 5 deletions adagrams/game.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,128 @@
import random

def draw_letters():
pass
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
}
list_letter_pool =[]
for letter, number in LETTER_POOL.items():
letter_quantity = letter * number
list_letter_pool.append(letter_quantity)


letter_string = ''.join(list_letter_pool)
Copy link

Choose a reason for hiding this comment

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

Nice use of join to flatten/combine the string!


# Available letters with frequencies list
available_letters = list(letter_string)
Comment on lines +32 to +41
Copy link

Choose a reason for hiding this comment

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

In this function, it looks like you're building a list of lists in list_letter_pool. Later on, you flatten list_letter_pool into a single list of strings in available_letters.

I think it might be worth thinking about how to refactor this logic-- while we iterate through LETTER_POOL.items(), is there a way we can create available_letters (without list_letter_pool)?


hand = []

# Returns a list of 10 random letters
Copy link

Choose a reason for hiding this comment

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

[nit] This comment should be indented correctly

while len(hand) != 10:
random_letter = random.choice(available_letters)
hand.append(random_letter)
for letter in available_letters:
if letter in hand:
available_letters.remove(letter)
Comment on lines +46 to +51
Copy link

@tildeee tildeee Mar 31, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Your logic ends up being a little bit buggy! I noticed this because when I ran your tests (I run them like 50 times as I grade lol), sometimes 5 tests failed, sometimes 6 tests failed for me.

I spent some time trying to learn about the bug and this is what I found. While len(hand) != 10, you want to:
- pick a random letter (random_letter)
- check if that random letter is available (aka in available_letters)
- Only if that random letter is available, then do we add it to the hand and remove it from available_letters

Your code currently does something else. While len(hand) != 10, it:
- pick a random letter (random_letter)
- always adds the random_letter to the hand
- Loops through all available_letters
- If the available letter is in the hand, then it remove it from available_letters

The key difference is when you add to hand and when you remove from available_letters

This is a fun bug to fix and an interesting refactoring of your code! Let me know what questions come up.

return hand

def uses_available_letters(word, letter_bank):
pass


def uses_available_letters(word, hand):
# Changes user's input to uppercase
word = word.upper()
# Returns true if all the letters in word are in the hand
for letter in word:
if letter in hand and word.count(letter) < hand.count(letter):
continue
elif letter not in hand or word.count(letter) > hand.count(letter):
return False
Comment on lines +60 to +64
Copy link

Choose a reason for hiding this comment

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

Good use of continue. In this case, I think our code still works if we get rid of the first if and only check the elif case (as an if case). For example, this still passes the uses_available_letters tests:

    for letter in word:
      if letter not in hand or word.count(letter) > hand.count(letter):
        return False

return True


def score_word(word):
pass

score = 0
bonus_point_list =[7,8,9,10]

points_value = {'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
} # Gets the score of the word
Copy link

Choose a reason for hiding this comment

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

[nit] This ending brace should be indented, and it might make sense to put the comment on its own line.

for letter in word.upper():
score += points_value[letter]
if len(word) == 0:
score = 0
if len(word) in bonus_point_list:
Copy link

Choose a reason for hiding this comment

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

I actually really like how you made and used bonus_point_list. To be honest, if len(word) >= 7 would also pass the tests, which is a little more straightforward/direct... But if we ever updated this project, and needed to update the bonus point lengths, we would only need to update the one bonus_point_list variable.

score = score + 8
Copy link

Choose a reason for hiding this comment

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

Small suggestion: score += 8 works here too

return(score)
Copy link

Choose a reason for hiding this comment

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

[nit] I think it would be better and more consistent code style to return score without parens and with a space.



def get_highest_word_score(word_list):
pass
# Stores the word and it's score
word_value_dictionary = {}

# Stores the scores of the words in
score_list = []
# Scores each word in the word list and stores it a dictionary
for word in word_list:
score = score_word(word)
word_value_dictionary[word] = score
# Gets the highest score
highest_score = max(word_value_dictionary.values())
# Gets the word with the highest score and returns it in a tuple
for word,value in word_value_dictionary.items():
if value == highest_score:
score_list.append(word)
score_list.append(highest_score)

return tuple(score_list)
Comment on lines +122 to +127
Copy link

Choose a reason for hiding this comment

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

Nice work! It seems like the details about the tie-breaking is what's next in this project. From what I can tell, I think that your next step here could be:

  1. You've found the highest_score and all of the word,values that match the highest_score. Instead of appending them to score_list, maybe there's another way of storing them.
  2. You want to store the tie-breakers because you eventually want to look at their word lengths. Just like how you found highest_score, you can find the smallest word and also all words that have a length of 10.