8
\$\begingroup\$
public static class StringHelpers
{
    public static string CapitalizeEachWord(this string sentence)
    {
        CultureInfo cultureInfo = Thread.CurrentThread.CurrentCulture;
        TextInfo textInfo = cultureInfo.TextInfo;

        if (String.IsNullOrEmpty(sentence))
        {
            return String.Empty;
        }

        return textInfo.ToTitleCase(sentence);
    }

    public static string CapitalizeFirstWord(this string sentence)
    {
        if (String.IsNullOrEmpty(sentence))
        {
            return String.Empty;
        }

        var words = sentence.Split();
        StringBuilder resultBuilder = new StringBuilder();

        var letters = words[0].ToCharArray();
        List<string> lettersHolder = new List<string>();
        lettersHolder.Add(letters[0].ToString().ToUpper());
        for (int i = 1; i < letters.Length; i++)
        {
            lettersHolder.Add(letters[i].ToString().ToLower());
        }

        foreach (var letter in lettersHolder)
        {
            resultBuilder.Append(letter);                   
        }

        resultBuilder.Append(" ");

        for (int i = 1; i < words.Length; i++)
        {
            resultBuilder.Append(words[i].ToLower());
            resultBuilder.Append(" ");
        }

        return resultBuilder.ToString();
    }
}

The first method, is supposed to correctly capitalize each word in a sentence. I don't think there's much room for improvement in that one.

The second method, seems a little confusing and even after I've written it and tested it, I cannot seem to fully put what it's doing into my buffer. That's a sign of code smell.

If I kinda get it now, what will happen down the line when I have to change it or something.

\$\endgroup\$
  • \$\begingroup\$ ToTitleCase() won't uncapitalize words that are in all caps. You should call it on the .ToLower() version of the string. \$\endgroup\$ – Bobson Apr 12 '13 at 16:26
10
\$\begingroup\$

You're right, your second method is way too complicated. It also doesn't handle the case where the string starts with a space (in which case words[0] will be the empty string and letters[0] will be out of bounds). Since all your method seems to be doing is to capitalize the first letter of the string and lower case everything else, why not just do that?

public static string CapitalizeFirstWord(this string sentence)
{
    if (String.IsNullOrEmpty(sentence))
    {
        return String.Empty;
    }

    String firstLetter = sentence.Substring(0,1);
    String rest = sentence.Substring(1);
    return firstLetter.ToUpper() + rest.ToLower();
}
\$\endgroup\$
  • \$\begingroup\$ Wow, why didn't I think of that! :D \$\endgroup\$ – Sergio Tapia Feb 21 '11 at 17:32
9
\$\begingroup\$

I don't like how they will convert a null into an empty string. Seems like a good way to hide errors until they pop up later. I would do something like:

public static string CapitalizeEachWord(this string sentence)
{
    if (String.IsNullOrEmpty(sentence))
    {
        return sentence;
    }

    CultureInfo cultureInfo = Thread.CurrentThread.CurrentCulture;
    TextInfo textInfo = cultureInfo.TextInfo;

    return textInfo.ToTitleCase(sentence);
}
\$\endgroup\$
  • \$\begingroup\$ You could just do if (String.IsNullOrEmpty(sentence)) { return sentence; } \$\endgroup\$ – MattDavey Apr 12 '13 at 8:21
  • \$\begingroup\$ @MattDavey Good point. I've updated my answer to include that. \$\endgroup\$ – ICR Apr 12 '13 at 10:05
3
\$\begingroup\$

How about adding a parameter of type CultureInfo to the first method?

That way consumers of your method can decide which CultureInfo to use.

\$\endgroup\$

Your Answer

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