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

Tigers, SAMIYA AND TANIL #72

Open
wants to merge 5 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
151 changes: 147 additions & 4 deletions adagrams/game.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,154 @@
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():
pass
letters = []
remaining_letters_pool = {}

if len(letters) < 11:

Choose a reason for hiding this comment

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

This is a redundant if statement. Given the fact that you've initialized letters to be an empty list on line 31, the length will always be less than 11! Therefore we can remove this if statement.

#getting the remaining amount of letters in the letter pool
for dict in LETTER_POOL:
for key, value in dict.items():
if key not in remaining_letters_pool and value > 0:
remaining_letters_pool[key] = value

Choose a reason for hiding this comment

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

If I'm reading this correctly, you are copying over all the values from LETTER_POOL into remainig_letters_pool. We have ways to do that which have already been optimized for us. You could have simply used the deepcopy() function to copy the dictionary over.

#adding random letter to the length of the letter list if it's less than ten
while len(letters) < 10:
random_letter = random.choice(list(remaining_letters_pool))
##Removing the used letter from the remaining letter pool
for letter in list(remaining_letters_pool):

Choose a reason for hiding this comment

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

If you do the suggestion from the last comment, you know for certain that the random_letter you get will be a key in the remaining letters pool. Therefor we don't have to loop through the list of keys again here. We can just access each key normally, comfortable in the fact that we are working with a key that exists.

if remaining_letters_pool[letter] == 0:
remaining_letters_pool.pop(letter)
elif random_letter == letter:
remaining_letters_pool[letter]-= 1
letters.append(random_letter)

Choose a reason for hiding this comment

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

Following the advice from above, we can simplify this into a single if statement by simply checking if the random_letter has values left in the dictionary:

 if remaining_letters_pool[random_letter] > 0:
      remaining_letters_pool[random_letter] -= 1
      letters.append(random_letter)

return letters


def uses_available_letters(word, letter_bank):
pass
dict_word={}
word=word.upper()
word_list=[]
values_dict=[]
#Creating a list of each letter that is a string
for letter in word:
word_list.append(letter)
#Assigning a boolean to each letter if it appears in the list
for letter in word_list:
if letter in letter_bank:
freq_letter=word_list.count(letter)
freq_letter_bank=letter_bank.count(letter)
if freq_letter == freq_letter_bank:
dict_word[letter]=True
elif freq_letter>freq_letter_bank:
dict_word[letter]=False
elif freq_letter<freq_letter_bank:
dict_word[letter]=True
else:
dict_word[letter]=False
#Adding all of the booleans values for the given word to a list
for key,value in dict_word.items():
values_dict.append(value)
if False in values_dict:
#if there are any False values in the dict then return False
return False
else:
return True

Choose a reason for hiding this comment

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

I see what y'all were trying to do here, and the code works and shows a good understanding of lists and dictionaries, but there's a simpler way to approach this problem and it starts with creating a deep copy of the letter bank. That would allow us to have a copy of the letter bank that we can modify without modifying the original. From there, you could simply loop through each letter in the word and check to see if it is in the copied letter_bank. If it is in there, remove the letter from the letter bank and move to the next one (That'll take care of duplicate letters). If it isn't there, we can immediately return false. That would look something like this:

 for letter in word:
      if letter in letter_bank_copy:
           letter_bank_copy.remove(letter)
     else: 
           return False
 return True


def score_word(word):
pass
#creating a new dictionary assinging values to each letter
letter_value={'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,}

total_sum=0
#Test case sensitivity
cap_word=word.upper()
if len(word) > 6:
total_sum=8
for letter in cap_word:
total_sum+=letter_value[letter]
return total_sum

Choose a reason for hiding this comment

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

This looks great! My only suggestion would be to make the letter_value dictionary a constant outside of these functions.


def get_highest_word_score(word_list):
pass
highest_score=0
words=[]
word_scored={}
#Creating a dictionary for each word with the key pair value word and score
for word in word_list:
word_scored[word]=score_word(word)
#Getting highest score
for value in word_scored.values():
if value > highest_score:
highest_score=value

#get words with highest_score
for word in word_list:
if word_scored[word]==highest_score:
words.append(word)

#evaluate tie-breaks
if len(words) > 1:
shortest_word_len = 10
shortest_word = ''
for word in words:
if len(word) == 10:
return (word, highest_score)
elif len(word) < shortest_word_len:
shortest_word_len = len(word)
shortest_word = word
return (shortest_word, highest_score)

#only one winner
else:

return (words[0], highest_score)

Choose a reason for hiding this comment

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

I once again get where y'all are coming from with this code, and it does look like it works, but there are a few strategies you could use to make this code more efficient/easier to read. The biggest tip I have with something like this is to recognize that you can do all the comparisons as you move through the list. I actually really like what y'all did to find the highest score and any words that reach that highest score, but it is somewhat unnecessary. A more efficient algorithm would start by creating a tuple to hold the highest scoring word and its value. Then you go through each word, score it, and then compare the score of that word with what's in the tuple using the for loops to figure out which one is correct. Doing the comparisons as you go along will make everything just that much simpler.