Is the string a pangram?

Question

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

public static class StringExtensions
{
    public static IEnumerable<char> EnglishAlphabet
    {
        get
        {
            for (char i = 'a'; i <= 'z'; i++)
            {
                yield return i;
            }

            yield break;
        }
    }

    /// <summary>
    /// Check if the source string is a pangram.
    /// </summary>
    /// <remarks>
    /// The default dictionary is the english one.
    /// </remarks>
    public static bool IsPangram(this string source)
    {
        return source.IsPangram(EnglishAlphabet);
    }

    /// <summary>
    /// Check if the source string is a pangram.
    /// </summary>
    /// <remarks>
    /// A pangram is a string that uses every letter in the alphabet.
    /// </remarks>
    public static bool IsPangram(this string source, IEnumerable<char> alphabet)
    {
        var lowerAlphabet = new string(alphabet.ToArray()).ToLower();
        IDictionary<char, bool> 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.

Answer 1

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.

Answer 2

Just a small point:

public static IEnumerable<char> EnglishAlphabet
{
    get
    {
        for (char i = 'a'; i <= 'z'; i++)
        {
            yield return i;
        }

        yield break;
    }
}

Strings already implement IEnumerable<char>. You could replace all this code with:

public readonly static IEnumerable<char> EnglishAlphabet = "abcdefghijklmnopqrstuvwxyz";

Answer 3

I would suggest that you consider using an immutable set:

static readonly ImmutableHashSet<char> alphabet = 
  ImmutableHashSet<char>.Empty.Union("abcdefghijklmnopqrstuvwxyz");
static bool IsPangram(string s)
{
  return alphabet.Intersect(s.ToLower()).Count == alphabet.Count;
}

Keep it short and simple; use off-the-shelf parts.

Answer 4

As others have pointed out you only have to check the number of distinct characters in the sentence. A one liner doing that using linq could be:

public static bool IsPangram2(this string text, int alphabetLength = 26)
{
  return text.ToUpper().Where(ch => char.IsLetter(ch)).Distinct().Count() >= alphabetLength;
}

As stated in a comment the above solution does not handle accented letters so a better solution may be:

private static string DefaultCharacters = "ABCDEFGHIJKLMNOPQRSTUVWXYZ";

public static bool IsPangram3(this string text, string validCharacters = null)
{
  text = text.ToUpper();
  validCharacters = validCharacters?.ToUpper() ?? DefaultCharacters;
  return !validCharacters.Any(ch => !text.Contains(ch));
}

Answer 5

Since you're (apparently) already changing to a version that uses a set (and its size) instead of a dictionary, I think it's also worth considering letting the user just pass the size of the alphabet they care about.

Using this your convenience overload would be just:

public static bool IsPangram(this string source)
{
    return source.IsPangram(26);
}

You really only care about the size of the alphabet anyway, so you might as well just pass the size. In the specific case of the English alphabet, it's quick and easy to generate all the lower-case letters, but for many other alphabets, this is a more difficult task--for example, for French or Spanish lower-case letters, it looks like you need to specify around 8 separate ranges of characters apiece (but it seems safe to assume French and Spanish speakers know the number of letters in their alphabets just as well as English speakers do). In short, passing the number is a lot easier than passing all the elements (which you really don't care about anyway).

Answer 6

Make it short, make it regex:

public static bool IsPangram(this string input)
{
    const int alphabetLength = 26;
    return Regex.Matches(input, @"[a-z]", RegexOptions.IgnoreCase)
        .Cast<Match>()
        .Select(m => m.Groups[0].Value.ToLower())
        .Distinct()
        .Count() == alphabetLength;
}

Answer 7

Without a line by line review
Just an implementation of what Janos answered
Give the check to Janos

    private HashSet<int> panaHash = new HashSet<int>();
    int cI;
    public bool IsPangram(string source)
    {
        panaHash.Clear();
        foreach (char c in source.ToLower())
        {
            cI = (int)c;
            if (cI < 97)   // not going to change in your life time 
                continue;
            if (cI > 122)  // not going to change in your life time 
                continue;
            panaHash.Add(cI);
            if (panaHash.Count == 26)
                return true;
        }
        return false;
    }

ran some time test and this was the fastest

me 1924
RegX 27436
Lippert 4587
Question 2357