Carmen and Nina - C18 Cheetahs #78
Conversation
There was a problem hiding this comment.
Good work on this project Carmen and Nina! There is some great code in here, good use of lists and dictionaries and very easy to read functions. However, this project is yellow due to an issue in Wave 1. Wave 1 passes the tests, but will always return the same set of letters for the letter bank. One of the important parts of this project was to play test the code (steps 3 & 4 in the project development workflow). The tests only check to see if this function produces a hand of letters with the right distribution, it doesn't check to see if running the function multiple times is producing a random list. The way to find this bug is through play testing. Let's discuss this at our next 1:1.
| #Pair Plan! | ||
| Learning styles: We both learn by doing. Once we learn a concept, we learn by applying. | ||
| Prefer receiving feedback like: Upfront, honest, straightforward | ||
| Communication skills we want to improve: Communicating ideas more, speaking up more | ||
| After Tuesday afternoon co-working, we'll check in about progress and sync up schedules if needed and consider office hours if need. If we're really stuck or struggling then we should ask for help! |
| for key, value in bag_of_letters_dict.items(): | ||
| for letter in range(value): | ||
| bag_of_letters_list.append(key) |
There was a problem hiding this comment.
Great way to generate the bag of letters!
| while len(hand) < 10: | ||
| for letter in bag_of_letters_list: | ||
| hand.append(letter) | ||
| letter_of_index = bag_of_letters_list.index(letter) | ||
| bag_of_letters_list.pop(letter_of_index) | ||
| break | ||
| return hand |
There was a problem hiding this comment.
This is really close to a working solution, but it will always produce the same list of letters (['A', 'A', 'A', 'A', 'A', 'A', 'A', 'A', 'A', 'B']). The primary issue is that there is no randomization in this solution. The while loop will run 10 times, and each time through the loop, the inner loop runs once. The inner loop will start with the first letter in bag_of_letters_list, append that letter to hand, remove the letter from the list, and then break out of the inner loop. This has the effect of appending the first letter from the list 10 times, which results in a hand of 9 'A's and 1 'B'.
To generate a random hand of letters, instead of the for loop on line 56, you could use something like random.randint(0,len(bag_of_letters)) to pick a random index in the list. The letter at that index would be added to the hand and then removed from bag_of_letters_list with pop. (random.randint)
|
|
||
| for letter in word: | ||
| word_list.append(letter) | ||
| if letter in letter_bank and word_list.count(letter) == letter_bank.count(letter): |
There was a problem hiding this comment.
This passes our tests, but there is a bug here. If letter_bank has multiple copies of the same letter, even if all of those letters are used in the word, the first time this loop tests one of those letters, this conditional will be False, because the count in word_list will be less than the count in letter_bank. Consider this examples:
- word = "dog" and letter_bank = ['d', 'o', 'o', 'g', 'x', 'x', 'x', 'x', 'x', 'x']. The second iteration through this loop, 'o' will be added to
word_list, but there will only be one 'o' in word list, so the count here will not be equal.
Changing the conditional to <= would fix this issue, because then it would be checking if there are enough letters in letter_bank, rather than the exact same number.
| if letter in letter_bank and word_list.count(letter) == letter_bank.count(letter): | |
| if letter in letter_bank and word_list.count(letter) <= letter_bank.count(letter): |
| else: | ||
| true_false.append(False) |
There was a problem hiding this comment.
There is a possible short circuit here. If the code ever reaches this point, the word is not valid, so it's possible to just return False here, rather than continuing to check all of the letters.
| else: | |
| true_false.append(False) | |
| else: | |
| return False |
| for letter in word: | ||
| for key, value in scoreboard.items(): | ||
| if letter in value: | ||
| score += key |
There was a problem hiding this comment.
This is nominally an O(n) solution, but only because the inner loop is not growing in relation to the input size. For every letter in word, this solution will have to loop through all of the letters in scoreboard in order to find the right key-value pair. This doesn't take advantage of dictionary lookup as an O(1) operation. Using a scoreboard dictionary where each letter is a key and the value is the point value would result in a more efficient solution, because then there would be no need for an inner loop (scoreboard[letter] would return the value for the letter). In the worst case scenario, if all of the letters of word are 'Z', the inner loop in this solution will have to do 26 comparison operations, but if scoreboard is set up as {'A':1, 'B':3 ... 'Z':10}, the inner loop is not needed and there's only one operation needed to find the value for the letter.
| winner_word = '' | ||
| for word in word_list: | ||
| word_score[word] = score_word(word) |
| winner_word = key | ||
|
|
||
| tuple = (winner_word, winner_score) |
There was a problem hiding this comment.
Great progress on this wave! This algorithm works well to find the top score when there are no ties.
No description provided.