Olena Rostotskyy and Joanna Parisi Adagram project c17 otter class#35
Olena Rostotskyy and Joanna Parisi Adagram project c17 otter class#35olenarostotskyy wants to merge 9 commits intoada-c17:masterfrom
Conversation
olenarostotskyy
changed the title
jbieniosek
left a comment
jbieniosek
left a comment
There was a problem hiding this comment.
Great work Olena and Joanna! Very clean, readable code. Nice work with the frequent commits. This project is green.
| if len(letter_list) < 10: | ||
| letter_list.append(letter) | ||
|
|
||
| return letter_list |
There was a problem hiding this comment.
This function will pass the tests, but the letter distribution is not correct. letter_key_list will contain all of the letters only once, and then the loop starting on line 41 will pick the first 10. Essentially, this is a letter distribution of 1 tile per letter.
| pool = deepcopy(letter_bank) | ||
| pool = [p.upper() for p in pool] # convert each element in a list to uppercase |
There was a problem hiding this comment.
Both of these lines are creating a new list. They can be combined:
| pool = deepcopy(letter_bank) | |
| pool = [p.upper() for p in pool] # convert each element in a list to uppercase | |
| pool = [p.upper() for p in letter_bank] # convert each element in a list to uppercase |
In addition, a deep copy is not necessary, a shallow copy, ie letter_bank.copy() would work here as well to create a copy of the list. The letters in the letter bank are immutable strings. A deep copy is only needed when the contents of the list are mutable, and the original data needs to be preserved.
| if letter.upper() not in pool: | ||
| return False | ||
| else: | ||
| pool.remove(letter.upper()) |
| list_4 = ["F", "H", "V", "W", "Y"] | ||
| list_5 = ["K"] | ||
| list_6 = ["J", "X"] | ||
| list_7 = ["Q", "Z"] |
There was a problem hiding this comment.
See feedback in Adagrams PSE for notes on optimization.
| for k,v in dict_of_words.items(): | ||
| if v>highest_score: | ||
| highest_score=v | ||
| highest_score_words.clear() | ||
| highest_score_words.append(k) | ||
| elif v==highest_score: | ||
| highest_score_words.append(k) |
| for e in highest_score_words: | ||
| if len(e) >=10: | ||
| highest_score_word=e | ||
| break | ||
| elif len(e)<len(highest_score_word): | ||
| highest_score_word=e |
There was a problem hiding this comment.
Fantastic work with this algorithm! Small note, I recommend avoiding single letter variable names - more descriptive variable improve readability.
| elif len(e)<len(highest_score_word): | ||
| highest_score_word=e | ||
|
|
||
| result=tuple([highest_score_word,highest_score]) |
There was a problem hiding this comment.
The tuple here can be built directly, no need to create a list and then convert it to a tuple.
| result=tuple([highest_score_word,highest_score]) | |
| result=tuple(highest_score_word,highest_score) |
| # while count <v: | ||
| # pool.append(k) | ||
| # count+=1 |
There was a problem hiding this comment.
This would work to create a letter pool with the correct distribution.
| # while counter <10:# creating random index from 0 to 10 | ||
| # random_inex=random.randint(0, len(pool)-1) | ||
| # hand.append(pool[random_inex]) #adding randomly picked letter to our hand (list of 10) | ||
| # pool.pop(random_inex) #removing the same element from the pool | ||
| # counter+=1 |
3 participants
No description provided.