Python program to find the most frequent letter in a text

Question

Given a text like "Hello World!" the program outputs the most frequent letter, in this case the "l".

If there are two letters with the same frequency, it outputs the first in alphabetical order, for example in the text "One" it outputs "e".

In addition upper and lower letters are counted as the same letter.

How could be this code improved?

def most_frequent_letter(text: str) -> str:

    letters_dict = {}
    text = text.lower()
    text = re.sub("[^a-z]","",text)

    for letter in text:
        if letter in letters_dict:
            letters_dict[letter] += 1
        else:
            letters_dict[letter] = 1

    value_to_key = collections.defaultdict(list)
    for key_letter, frecuency_value in letters_dict.items():
        value_to_key[frecuency_value].append(key_letter)

    letters = list(value_to_key.values())
    frecuencies = list(value_to_key.keys())
    return (sorted(letters[frecuencies.index(max(frecuencies))])[0])

Answer 1

If you are going to use the collections module at all (as you did for collections.defaultdict), then why not use collections.Counter, which offers a most_common() method?

Answer 2

For the most part your code is okay, however, reading what it is that you want it to do here's what I came up with...

#!/usr/bin/env python3

import re
import typing


def most_frequent_letter(text: str) -> typing.Tuple[int, list]:
    text = re.sub('[^a-z]', '', text.replace(' ', '').lower())

    letter_counts = {}
    for letter in text:
        letter_counts[letter] = letter_counts.get(letter, 0) + 1

    counted_letters = {}
    for letter, counter in letter_counts.items():
        counted_letters[counter] = counted_letters.get(counter, []) + [letter]

    max_key = max(counted_letters.keys())
    return max_key, sorted(counted_letters[max_key])


if __name__ == '__main__':
    counted, letters = most_frequent_letter('spam ham jello spam')
    print("{counted} -> {letter}".format(
        counted = counted,
        letter = letters[0]))

... while I don't know that it's an improvement overall it does return results that may be of use...

3 -> a

Note if ya want the whole list use the following instead under that last if statement...

# ...
    print("{counted} -> {letters}".format(
        counted = counted,
        letters = letters))
# ... Should output...
#     3 -> ['a', 'm']

... while also not counting spaces as letters; which is the only thing that seemed missing from your implementation.

I didn't see any need to use if statements (within the function) so didn't include'em, because dictionaries have a handy get method that allows for defaulting nonexistent key value pares. And I didn't see any use in using collections though it was tempting to use @200_success's suggestion.

One thing about your code that I do still question though is why are ya wrapping the returned value within parentheses?

Answer 3

Your code appears to be correct, but I have a few comments.

First, using the typing, re, and collections modules is perhaps somewhat overkill for what you are trying to achieve. For small programs, it's best to make use of the built-in functions, and methods of build-in types, rather than to import modules, if possible. In particular, the string methods can be useful, and I've found the max, map, all, and any functions to be handy.

Second, while it wasn't specified what the function should do given an empty string, it would be prudent to handle this case. Currently, your code will throw an error; I think it would make sense to return the letter 'a'.

Third, parts of your code are a bit difficult to read. In particular, this expression

(sorted(letters[frecuencies.index(max(frecuencies))])[0])

required some concentration to understand. I'm not sure how this could be improved. I think the problem is that by this point I've lost track of what the data-structures look like.

---

That's it for comments. To finish, here is an approach to the problem that I think would be considered quite Pythonic:

def most_frequent_letter(text):
    """Return the first letter of the alphabet that occurs jointly most 
    often in the given string. Case is ignored."""
    alphabet = 'abcdefghijklmnopqrstuvwxyz'
    return max(alphabet, key=text.lower().count)

The max function will return the first maximal element in the list, where the value being maximised is the result of applying the function key. So in this case, it will return the alphabetically first letter with the (joint) highest count in the lowercased text.

You could import the latin alphabet from the string module, if you'd rather not type it out:

from string import ascii_lowercase

---

I would work on:

• becoming more familiar with the built-in methods and functions;
• writing code with the human reader in mind: try to keep the state of variables simple and self-evident. Ideally a line of code should be somewhat comprehensible in isolation without having to use working memory to mentally build up the state of the variables from the previous lines of the program.