Conversation
kendallatada
left a comment
kendallatada
left a comment
There was a problem hiding this comment.
Hi Fena and Roshni! Your submission has been scored as green. Please check your code to see the comments I left. Let me know if you have any questions. Nice work! :)
|
|
||
| def draw_letters(): | ||
| pass | ||
| #PSEUDOCODE |
There was a problem hiding this comment.
Already chatted with Fena about this, but want to make sure it gets noted in your code too. Love how you're using comments to document what your code is doing! They are a great tool to use while you're still building out an algorithm to solve a problem. Once you have a solution, it's important to only have comments where it's necessary for readability. I'd recommend checking out the Comments section in PEP 8 to get a good understanding of style conventions around this.
| letter = random.choice(a_z_list) | ||
| #catching scenario if current letter distrubution is zero | ||
| if letter_frequency[letter] == 0: | ||
| continue |
There was a problem hiding this comment.
Right now, it's possible for a letter with a frequency of zero to be selected. Worst case scenario, the random.choice() function keeps accidentally selecting an unavailable letter and this loop runs forever. How can you improve this solution to be O(1)?
| a_z_list = [key for key in LETTER_POOL] | ||
|
|
||
| #making copy of available letters w/ distribution of letters | ||
| letter_frequency = copy.deepcopy(LETTER_POOL) |
There was a problem hiding this comment.
Nice usage of copy.deepcopy() to prevent making changes to the LETTER_POOL constant.
| for letter in letter_bank: | ||
| #if statement for if the letter in the letter bank is in our letter freq dict | ||
| #then talley one, else create new key-value pair | ||
| if letter in letter_frequency: |
There was a problem hiding this comment.
Ooo this if-else block would be a great place to implement a try-except statement to adhere to EAFP (Easier to Ask Forgiveness than Permission). Alternatively, you could look into the get method to handle keys that don't exist. Here's a resource for that here.
| letter_frequency[letter] = 1 | ||
|
|
||
| #for looping through user's word to check if letter is available in letter freq dict | ||
| for letter in word: |
There was a problem hiding this comment.
This is a really good solution! I would challenge you to think about how you could refactor this code to only use one for loop. :)
| for key, value in SCORE_CHART.items(): | ||
| #looping through letter array to determine score of current letter | ||
| for available_letter in value: | ||
| if letter == available_letter: |
There was a problem hiding this comment.
This is an interesting solution! Three for loops is going to be pretty inefficient though, so I would challenge you to think about how you could refactor this solution to only use 1 for loop.
I also want to comment on this third for loop. Rather than looping over each letter in value and checking if that letter is our current letter, we could use the in keyword to eliminate this for loop. So we could have if letter in value:. This will technically still be an O(n) operation because checking if an item is in a list is O(n). However, if we update the data structures from lists to sets, we get a time complexity of O(1). Like I said earlier, this whole solution can be refactored down to a single for loop but I did want to mention the in keyword and using sets to improve time complexity for this current solution.
| max_pair = None | ||
|
|
||
| #looping through available words/scores | ||
| for pair in max_score_list: |
There was a problem hiding this comment.
This is a good solution! This loop has to iterate over each word again though which is repetitive work. Think about how you can combine the work from the first loop with the work being done in this second for loop. This will improve readability, cut down on lines of code, and take this solution from O(2n) to O(n).
3 participants
No description provided.