HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Is the string a pangram?

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
pangramthestring

Problem

The following code checks if a string is a pangram or not:

public static class StringExtensions
{
    public static IEnumerable EnglishAlphabet
    {
        get
        {
            for (char i = 'a'; i 
    /// Check if the source string is a pangram.
    /// 
    /// 
    /// The default dictionary is the english one.
    /// 
    public static bool IsPangram(this string source)
    {
        return source.IsPangram(EnglishAlphabet);
    }

    /// 
    /// Check if the source string is a pangram.
    /// 
    /// 
    /// A pangram is a string that uses every letter in the alphabet.
    /// 
    public static bool IsPangram(this string source, IEnumerable alphabet)
    {
        var lowerAlphabet = new string(alphabet.ToArray()).ToLower();
        IDictionary alphabetCharacters = lowerAlphabet.ToDictionary(c => c, c => false);
        var lowerSource = source.ToLower();

        foreach(char character in lowerSource)
        {
            alphabetCharacters[character] = true;
        }

        return !alphabetCharacters.Values.Contains(false);
    }
}


Do you see anything wrong/weird in it? Any feedback (style, naming, performance) is more than welcome.

Solution

Use a set instead of a dictionary

The current algorithm sets the value to true for each character it finds,
and at the end it checks if there is anything still unset. This is a bit tedious, and inefficient. Checking if any value is false is an \$O(n)\$ operation.

A simpler and more natural solution would be using a set data structure (HashSet in C#). You could add characters that you found, and in the end check the size of the set, and compare it with the size of the alphabet.

If the input may contain letters not in the alphabet, then before adding letters to the found set, don't forget to check the letter is actually part of the alphabet. (Thanks @tym32167 for the reminder.)

Use a simple array instead of a dictionary

If the alphabet is always the English alphabet, then an interesting simple alternative is using a simple boolean array of size 'z' - 'a' + 1. As you iterate over the letters, you can derive the array index to use by letter - 'a'. It will be simpler and more storage efficient than using a dictionary.

Generate the alphabet once

The English alphabet is regenerated on each call of IsPangram.
This is unnecessary, as it never changes between calls.
You could generate it once and reuse.

Context

StackExchange Code Review Q#142650, answer score: 9

Revisions (0)

No revisions yet.