Heather M. & Esther A. adagrams project

[cathos]

No description provided.

[kelsey-steven-ada]

Great work Heather & Esther! Really interesting, concise approaches to the functions. I've added some alternatives and suggestions to refactor for readability. Take a look, and let me know if you have any questions!

[kelsey-steven-ada]

We want to remove commented code from files when we're done testing.

[kelsey-steven-ada]

This is a really cool solution to condense filling the letters list to a single line! I would suggest breaking this apart to make it easier to read and understand. The PEP 8 style guide recommends limiting line to a max of 79 characters to make code easier to read, especially with multiple editor windows open: https://peps.python.org/pep-0008/#maximum-line-length. Outside of the style guide, when you're in situations where other people will be reading and working in the same code, the ability to quickly and easily comprehend what the code is doing is often more valuable than reducing line count. For list comprehensions like this, I would strongly suggest including a comment that describes what the line is doing.

[kelsey-steven-ada]

Since LETTER_POOL is only used by the draw_letters function, it would be reasonable to place this inside the function rather than as a constant. There are tradeoffs, this structure does clutter the function some, but it would keep the data as close as possible to where it's being used, and would mean other functions would have no way to access it to accidentally alter the contents.

[kelsey-steven-ada]

The feedback on line 3 about placing data in the function where its used applies here too.

[kelsey-steven-ada]

This is another line I'd recommend breaking for length:

Suggested change

	if letter not in letter_bank or (word_uppercase.count(letter) > letter_bank.count(letter)):
	letter_count_word = word_uppercase.count(letter)
	letter_count_bank = letter_bank.count(letter)
	if letter not in letter_bank or (letter_count_word > letter_count_bank):

[kelsey-steven-ada]

Since they are iterable and non-mutable, we could also use strings instead of tuples here:

Suggested change

	("A", "E", "I", "O", "U", "L", "N", "R", "S", "T") : 1,
	"AEIOULNRST" : 1,

[kelsey-steven-ada]

highest_index feels a little ambiguous in meaning to me here. Something along the lines of highest_scored_index or winning_index might describe the contents a little closer.

[kelsey-steven-ada]

Another approach would be to loop directly over the word_list and return the winning word itself:

Suggested change

	highest_index = 0
	for i in range(len(word_list)):
	if len(word_list[i]) == 10:
	return i
	elif len(word_list[i]) < len(word_list[highest_index]):
	highest_index = i
	return highest_index
	shortest_word = None
	for word in word_list:
	if len(word) == 10:
	return word
	elif not shortest_word or len(word) < len(shortest_word):
	shortest_word = word
	return shortest_word

[kelsey-steven-ada]

I recommend moving this initialization down to right above if len(highest_scoring_words) > 1: on line 130 to keep the creation of this data close to where it is used.

[kelsey-steven-ada]

I'd recommend whitespace between the for loop that finds the max scored words and the if statement that handles tie breaks & returns to help folks see the logical sections of the function at a glance.

[kelsey-steven-ada]

I like how you're using lists where you need to build up data then converting it to a tuple right when you need it.