0
\$\begingroup\$

I made a function that outputs the sum of the value of each letter in a word. For example if I input hello my function would output 52.

def words_to_marks(s):
    import string
    values = {}             # dictionary to store value of each alphabet
    values_of_words = []    # to store final value of words
    for index, letters in enumerate(string.ascii_lowercase, 1):     # creates the dictionary
        values[letters] = index
    final = []              # stores value of each number before adding
    for letter in s:
        letter_lower = letter.lower
        final.append(values[letter])    # appends the value of each letter into the list
    values_of_words.append(sum(final))  # sums all the values in the list
    submit = str(values_of_words)       # converts into string so I can remove the brackets around the list
    return submit[1:-1]


test = words_to_marks("friends")       
print(test)

I would really appreciate it if someone could point me to the right direction for how I should condense or improve my code.

\$\endgroup\$
1
  • 3
    \$\begingroup\$ Hello, you'll need to explain in your post how you map a letter to its numerical value, because there are a gazillion ways to do this :) \$\endgroup\$ – IEatBagels Nov 29 '19 at 19:52
3
\$\begingroup\$

Honestly, the initial words_to_marks function is full of "anti-patterns/bad smells".

When reviewing in steps:

  • instead of iterating over string.ascii_lowercase and generating/filling a dictionary of letter indices on each function call with:

    for index, letters in enumerate(string.ascii_lowercase, 1):     # creates the dictionary
        values[letters] = index
    

    create a predefined top-level dict lowercase_indices_map at once:

    import string
    
    lowercase_indices_map = {c:i for i, c in enumerate(string.ascii_lowercase, 1)}
    
  • converting each letter of the word with:

    for letter in s:
       letter_lower = letter.lower
    

    is completely redundant as the whole word can be lowercased at once s = s.lower()

  • and the "magic" like:

    values_of_words.append(sum(final))  # sums all the values in the list
    submit = str(values_of_words)       # converts into string so I can remove the brackets around the list
    return submit[1:-1]
    

    is just an unnecessary thing that you should avoid doing in future. The sum of letter indices is simply calculated with a single sum(...) call.


The whole idea fits in few lines of code:

import string

lowercase_indices_map = {c:i for i, c in enumerate(string.ascii_lowercase, 1)}

def sum_letters_indices(word):
    word = word.lower()
    return sum(lowercase_indices_map[char] for char in word)

Sample usage:

print(sum_letters_indices("hello"))    # 52
print(sum_letters_indices("friends"))  # 75
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.