-
Notifications
You must be signed in to change notification settings - Fork 89
Snow Leopards - Jessica Hebert & Jennifer Nouel #81
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,11 +1,144 @@ | ||
| from multiprocessing import pool | ||
| import random | ||
|
|
||
| def build_letter_pool(): | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see that this helper function returns the letter pool which keeps the other function bodies concise. I think when you have a large data structure like this, it is helpful to pull them out of the functions that use them so they don't get dominated by the data structure. However, since LETTER_POOL is a constant variable, you can have it as a global variable in game.py without wrapping it in a helper function. |
||
| 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 | ||
| } | ||
|
|
||
| return LETTER_POOL | ||
|
|
||
|
|
||
| def build_available_letters_list(LETTER_POOL): | ||
| available_letters = [] | ||
|
|
||
| for letter,frequency in LETTER_POOL.items(): | ||
| for i in range(frequency): | ||
| available_letters.append(letter) | ||
|
|
||
| return available_letters | ||
|
Comment on lines
+38
to
+44
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice helper function to keep draw_letters a single responsibility function and more concise 👍 |
||
|
|
||
|
|
||
| def draw_letters(): | ||
| pass | ||
| hand = [] | ||
| LETTER_POOL = build_letter_pool() | ||
| available_letters = build_available_letters_list(LETTER_POOL) | ||
| count_dict = {} | ||
|
|
||
| while len(hand) < 10: | ||
| random_letter = random.choice(available_letters) | ||
|
|
||
| if random_letter not in hand: | ||
| hand.append(random_letter) | ||
| count_dict[random_letter] = 1 | ||
| elif LETTER_POOL[random_letter] > count_dict[random_letter]: | ||
| hand.append(random_letter) | ||
| count_dict[random_letter] += 1 | ||
|
|
||
| return hand | ||
|
|
||
|
|
||
| def uses_available_letters(word, letter_bank): | ||
| pass | ||
| letter_bank_copy = letter_bank.copy() | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another Pythonic way you'll see to make a shallow copy is: |
||
| for letter in word.upper(): | ||
| if letter not in letter_bank_copy: | ||
|
Comment on lines
+68
to
+69
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's have a closer look at the time complexity involved with lines 68-69. Line 68: a for loop that loops as many times as there are letters in word - O(n) Rather than using a list of the letters in the hand, could we build a helper data structure (like a dictionary) that could let us look up if a letter was part of a user's hand? If the helper dictionary had keys that were the letters and the values were the number of letters in the hand, then you could leverage the fact that key look up in dictionaries is a constant O(1) operation. So on line 69, you can check if key (the letter in the hand) in helper_dictionary, which has constant look up time. Additionally, instead of using the remove method, you could decrement the value in the dictionary to keep track of how many letters have been played. Recall that the remove method on a list also has linear run time because it could have to iterate over the entire list before finally removing letter. |
||
| return False | ||
|
|
||
| letter_bank_copy.remove(letter) | ||
|
|
||
| return True | ||
|
|
||
|
|
||
| def build_score_chart(): | ||
| """ | ||
| |Letter | Value| | ||
| |:----------------------------:|:----:| | ||
| |A, E, I, O, U, L, N, R, S, T | 1 | | ||
| |D, G | 2 | | ||
| |B, C, M, P | 3 | | ||
| |F, H, V, W, Y | 4 | | ||
| |K | 5 | | ||
| |J, X | 8 | | ||
| |Q, Z | 10 | | ||
| """ | ||
|
|
||
| score_chart = { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since this helper just returns a constant variable, you can make score_chart a global constant called SCORE_CHART that is declared at the top of this file. |
||
| ("A", "E", "I", "O", "U", "L", "N", "R", "S", "T"): 1, | ||
| ("D", "G"): 2, | ||
| ("B", "C", "M", "P"): 3, | ||
| ("F", "H", "V", "W", "Y"): 4, | ||
| ("K"): 5, | ||
| ("J", "X"): 8, | ||
| ("Q", "Z"): 10 | ||
| } | ||
|
|
||
| return score_chart | ||
|
|
||
|
|
||
| def score_word(word): | ||
| pass | ||
| score_chart = build_score_chart() | ||
| score = 0 | ||
|
|
||
| if 7 <= len(word) <= 10: | ||
| score += 8 | ||
|
|
||
| for letters, points in score_chart.items(): | ||
| for char in word.upper(): | ||
| if char in letters: | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since To avoid increasing time complexity, you could refactor score_chart so that it is a dictionary where the keys are letters and the values are integers (neither keys or values would be list or tuple which both have linear time complexity for If you refactor score_chart as suggested, then you can leverage a dictionary's constant time look up to see if a key (letter) is in the dict. That means your dictionary would be longer, but if you declare it as a constant global variable at the top of the file, it won't clutter your method here and it's worth having a longer dict, but quicker look up time. |
||
| score += points | ||
|
|
||
| return score | ||
|
|
||
|
|
||
| def get_highest_word_score(word_list): | ||
| pass | ||
| highest_word_score = ("", 0) | ||
|
|
||
| for word in word_list: | ||
| current_word_score = (word, score_word(word)) | ||
| if current_word_score[1] > highest_word_score[1]: | ||
| highest_word_score = current_word_score | ||
|
|
||
| # tie breakers | ||
| if current_word_score[1] == highest_word_score[1]: | ||
| # if they're the same length pick the first word | ||
| if len(current_word_score[0]) == len(highest_word_score[0]): | ||
| continue | ||
|
|
||
| # or pick the word with a length of 10 | ||
| elif len(current_word_score[0]) == 10: | ||
| highest_word_score = current_word_score | ||
|
|
||
| elif len(highest_word_score[0]) == 10: | ||
| continue | ||
|
|
||
| # or pick the word with the fewest letters | ||
| elif len(current_word_score[0]) < len(highest_word_score[0]): | ||
| highest_word_score = current_word_score | ||
|
|
||
| return highest_word_score | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,7 @@ | |
|
|
||
| from adagrams.game import score_word, get_highest_word_score | ||
|
|
||
| #@pytest.mark.skip() | ||
| def test_get_highest_word_score_accurate(): | ||
| # Arrange | ||
| words = ["X", "XX", "XXX", "XXXX"] | ||
|
|
@@ -14,6 +15,7 @@ def test_get_highest_word_score_accurate(): | |
| assert best_word[0] == "XXXX" | ||
| assert best_word[1] == 32 | ||
|
|
||
| #@pytest.mark.skip() | ||
| def test_get_highest_word_score_accurate_unsorted_list(): | ||
| # Arrange | ||
| words = ["XXX", "XXXX", "XX", "X"] | ||
|
|
@@ -25,6 +27,7 @@ def test_get_highest_word_score_accurate_unsorted_list(): | |
| assert best_word[0] == "XXXX" | ||
| assert best_word[1] == 32 | ||
|
|
||
| #@pytest.mark.skip() | ||
| def test_get_highest_word_tie_prefers_shorter_word(): | ||
| # Arrange | ||
| words = ["MMMM", "WWW"] | ||
|
|
@@ -38,6 +41,7 @@ def test_get_highest_word_tie_prefers_shorter_word(): | |
| assert best_word[0] == "WWW" | ||
| assert best_word[1] == 12 | ||
|
|
||
| #@pytest.mark.skip() | ||
| def test_get_highest_word_tie_prefers_shorter_word_unsorted_list(): | ||
| # Arrange | ||
| words = ["WWW", "MMMM"] | ||
|
|
@@ -51,6 +55,7 @@ def test_get_highest_word_tie_prefers_shorter_word_unsorted_list(): | |
| assert best_word[0] == "WWW" | ||
| assert best_word[1] == 12 | ||
|
|
||
| #@pytest.mark.skip() | ||
| def test_get_highest_word_tie_prefers_ten_letters(): | ||
| # Arrange | ||
| words = ["AAAAAAAAAA", "BBBBBB"] | ||
|
|
@@ -62,6 +67,7 @@ def test_get_highest_word_tie_prefers_ten_letters(): | |
| assert best_word[0] == "AAAAAAAAAA" | ||
| assert best_word[1] == 18 | ||
|
|
||
| #@pytest.mark.skip() | ||
| def test_get_highest_word_tie_prefers_ten_letters_unsorted_list(): | ||
| # Arrange | ||
| words = ["BBBBBB", "AAAAAAAAAA"] | ||
|
|
@@ -73,6 +79,7 @@ def test_get_highest_word_tie_prefers_ten_letters_unsorted_list(): | |
| assert best_word[0] == "AAAAAAAAAA" | ||
| assert best_word[1] == 18 | ||
|
|
||
| #@pytest.mark.skip() | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since the changes in your test files are just to comment out the skip test decorate, you don't technically need to add and commit these changes (but it doesn't hurt anything to do so). You could just add and commit the changes in game.py and that would make your pull request more concise with just the logical changes you made. |
||
| def test_get_highest_word_tie_same_length_prefers_first(): | ||
| # Arrange | ||
| words = ["AAAAAAAAAA", "EEEEEEEEEE"] | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like this import is unused so you can delete it. Perhaps it was accidentally brought in by VSCode. There should be an option to turn off auto completion for imports to avoid bringing in stray, unused imports