Dragonfly - Sno Ochoa

[ra-choa]

I understand that wave 04 function could be made DRI, if possible, could I receive specific direction/ feedback to help condense that code section? Thank you!

[mikellewade]

Nice work on Adagrams, Sno!

[mikellewade]

Though this is functionally sound, let's think about the maintainability of this. Imagine if the client asked you to change the weights of 6 random letters. With this set up we are more likely to make some errors in the count. We should to go for a data structure that is more maintainable like a dictionary. We could store the letters as keys and their weights associated values. It could look like this:

LETTER_DICT = {
	    "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
	}

With this we could then use some looping to iterate over the dictionary to get an dynamically build LETTER_POOL.

[mikellewade]

Also something to consider when making constant variable, and variables in general, is the scope. The larger the scope the more functions and operations have access to them. This can create more chances for them to unintentionally changed. So, a question I tend to ask myself is "what needs access to the variable I am about to create?".

[mikellewade]

I would condense this to a few lines that way our codebase could look a more uniform since we did LETTER_POOL like this.

Suggested change

	SCORE_CHART = {
	"A" : 1,
	"E" : 1,
	"I" : 1,
	"O" : 1,
	"U" : 1,
	"L" : 1,
	"N" : 1,
	"R" : 1,
	"S" : 1,
	"T" : 1,
	"D" : 2,
	"G" : 2,
	"B" : 3,
	"C" : 3,
	"M" : 3,
	"P" : 3,
	"F" : 4,
	"H" : 4,
	"V" : 4,
	"W" : 4,
	"Y" : 4,
	"K" : 5,
	"J" : 8,
	"X" : 8,
	"Q" :10,
	"Z" :10
	}
	SCORE_CHART = {
	"A": 1, "E": 1, "I": 1, "O": 1, "U": 1,
	"L": 1, "N": 1, "R": 1, "S": 1, "T": 1,
	"D": 2, "G": 2,
	"B": 3, "C": 3, "M": 3, "P": 3,
	"F": 4, "H": 4, "V": 4, "W": 4, "Y": 4,
	"K": 5,
	"J": 8, "X": 8,
	"Q": 10, "Z": 10
	}

[mikellewade]

Since this variable is now being altered and modified we no longer need to use the python convention of naming constant variables.

Suggested change

	LETTER_POOL_copy = LETTER_POOL.copy()
	letter_pool_copy = LETTER_POOL.copy()

[mikellewade]

Operators like - should be surrounded with white spaces. Take a look at the PEP8 style guide on white spaces. In industry, code that doesn't follow an agreed upon code styles will need to be revised to ensure the codebase is neat and consistent.

Suggested change

	index = randint(0,len(LETTER_POOL_copy) -1)
	index = randint(0,len(LETTER_POOL_copy) - 1)

[mikellewade]

Love a frequency map! You could condense this into one line my using the .get method available for dictionaries. I would also like to suggest that you name you i into something more descriptive such as letter or char.

Suggested change

	for i in letter_bank:
	if i not in letter_bank_count:
	letter_bank_count[i] = 1
	else:
	letter_bank_count[i] += 1
	for letter in letter_bank:
	letter_bank_count[letter] = letter_bank_count.get(letter, 0) + 1

[mikellewade]

⭐️

[mikellewade]

Another approach you could take to this is use a cumulative weight. This would cut the list of 98 letters from our code. We would essentially make another dictionary cumulating the weights:

LETTER_POOL = {'A': 9, 'B': 2, 'C': 2, 'D': 4, 'E': 12}
cumulative_weights = {'A': 9, 'B': 11, 'C': 13, 'D': 17, 'E': 29} # Total Weight: 29

Letter	Count	Cumulative Weight
A	9	9 (1-9 → 'A')
B	2	11 (10-11 → 'B')
C	2	13 (12-13 → 'C')
D	4	17 (14-17 → 'D')
E	12	29 (18-29 → 'E')

We could then randomly select a number between 1 - total weight. After that we would loop through the cumulative_weights dictionary and if the random number we got is less than the cumulative weight we are looking at on the current iteration we will select that that letter.

Note: The chances of choosing any number between 1 - 9 from a pile of numbers 1 - 98 is the same as choosing one of the 9 A's from the pile of 98 letters.

This code could look something like this:

Click to view the draw_letters function

import random

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
}

def compute_cumulative_weights(letter_pool):
    cumulative_weights = {}
    total_weight = 0
  
    for letter, weight in letter_pool.items():
        total_weight += weight
        cumulative_weights[letter] = total_weight
      
    return cumulative_weights, total_weight

def draw_letters():
    cumulative, total_weight = compute_cumulative_weights(LETTER_POOL)
    selected_letters = []

    for _ in range(10):
        rand_value = random.randint(1, total_weight)  # Pick a random number
      
        for letter, cumulative_weight in cumulative.items():
            if rand_value <= cumulative_weight:
                selected_letters.append(letter)
                break

    return selected_letters

[mikellewade]

Is this section of code contributing as much as we think? Looking at the other two sections of code you have in this function do you think that this case is being covered somewhere else?

[mikellewade]

So, I know that you asked for guidance on how to condense this function. Let's think about how we could break this down into sub-problems, specifically two. The first one can be to make a list of the words of that tie for the highest score. The second, implement the tie breaking logic. There are also a few lines where you set winning_word to winning_word you don't need to do this since you are essentially asking it to point to the same data that its already pointing to. It seems like you were writing if statements to check for conditions when winning_word stays the same and for when you need to update it to contender_word. You just need to check for the latter. Also you can remove line 118 and just have return winning_word, winning_word_score on line 120instead since this will automatically return a tuple. So, breaking up the function into two sub problems and then only checking for conditions whenwinning_word` would change could lead to something like this:

def get_highest_word_score(word_list):
    top_score = 0
    tied_words = []

    for word in word_list:
        score = score_word(word)

        if score > top_score:
            top_score = score
            tied_words = [word]
        elif score == top_score:
            tied_words.append(word)
    
    top_word = tied_words[0]

    for word in tied_words:
        if len(word) == 10:
            return word, top_score
        elif len(word) < len(top_word):
            top_word = word
    
    return top_word, top_score

[mikellewade]

Suggested change

	result = (winning_word, winning_word_score)
	return result
	return winning_word, winning_word_score