1

I have a code to capitalize the first letter of every words except for one letter words.

The problem I have is if the last word of that string is one letter, it gives a index out of range exception. Which makes sense because the code array[i + 1] doesn't exist for the last letter.

static string UppercaseWords(string value)
{
    char[] array = value.ToLower().ToCharArray();
    // Handle the first letter in the string.

    array[0] = char.ToUpper(array[0]);
    // Scan through the letters, checking for spaces.
    // ... Uppercase the lowercase letters following spaces.
    for (int i = 1; i < array.Length; i++)
    {
        if (array[i - 1] == ' ' && array[i + 1] != ' ') 
        {
            array[i] = char.ToUpper(array[i]);
        }
    }
    return new string(array);
}

I'm just looking for something to get around that exception, or another way to do this.

Jimi
  • 29,621
  • 8
  • 43
  • 61
hat
  • 23
  • 2

3 Answers3

2

You could extract all the words (string parts separated by a white space) and convert to uppercase the first letter of the string part when the part length is > 1:

string input = "this is a sample, string with: some => 1 letter words ! a";

StringBuilder sb = new StringBuilder(input.Length * 2);
foreach (string word in input.Split())
{
    if (word.Length > 1) {
        sb.Append(char.ToUpper(word[0]));
        sb.Append(word.Substring(1));
    }
    else {
        sb.Append(word);
    }
    sb.Append((char)32);
}
Console.WriteLine(sb);

Prints:

This Is a Sample, String With: Some => 1 Letter Words ! a

Jimi
  • 29,621
  • 8
  • 43
  • 61
  • I didnt think about using the StringBuilder, with a verry verry long string this solution is better. (verry verry long) – Andreas Gustafsson Feb 02 '19 at 18:13
  • @Andreas Gustafsson When it comes to string concatenation, StringBuilder is the most efficient tool we have. It also prevents the accumulation of garbage strings. – Jimi Feb 02 '19 at 18:17
  • @Andreas Gustafsson See also here: [How come for loops in C# are so slow when concatenating strings?](https://stackoverflow.com/a/50071171/7444103). – Jimi Feb 02 '19 at 18:18
  • The char array is in that situation faster. You have first to split the string so allocation of a lot small strings. When you call substring, you again have to create another string. And at last. Your solution kills are whitespaces that are not ' ' (space). So any line break is turned into space. The original question didnt take care about other whitespaces nor interpuntions but in char array case it is easy to call .IsWhitespace(). With Split() method, this information is discarded. But I didnt any any benchmarks, I did only only assumptions in my mind. – Ondřej Kubíček Feb 02 '19 at 19:01
  • And of course the line `word[0].ToString().ToUpper() + word.Substring(1) : word) + (char)32` itself is another string concatenation (every '+' there is). With StringBuilder is better to use another `sb.Append((char)32);` (and using numeric constants instead of characters breaks readability) – Ondřej Kubíček Feb 02 '19 at 19:13
  • @Ondřej Kubíček Using an array is probably faster. It's the method used in most of the .Net source code that deals with string parsing. It needs to be tested, though. You are right about the string concatenation method I used in the code here: it's not that efficient. I've updated it. I didn't take care of line breaks because the original code doesn't: it's implied that the string is a single line of text. If it's not, it must be stated clearly in the question. Anyway, no answers here do. Btw, I didn't downvote your answer which, testing it, actually works. So, I'll upvote it instead. – Jimi Feb 02 '19 at 20:35
  • @Ondřej Kubíček Anyway, the balance between readability/maintainability and raw *speed* is a factor itself. – Jimi Feb 02 '19 at 20:36
  • @Jimi Now your answer looks a lot better, I upvote yours. I wrote that comment mainly because you were using StringBuilder with a lot of string concatenations in it so that StringBuilder efficiency was not that good. Now it is great. For readability and speed. And sorry for that code reviewing but what do you think about `word[0].ToString().ToUpper()` vs `char.ToUpper(word[0])`? I know it was not stated in the question that it should be efficient but you know for the future. – Ondřej Kubíček Feb 02 '19 at 22:06
1

You can improve condition so you will not ask for the (i+1)-th index.

if (array[i - 1] == ' ' && i + 1 < array.Length && array[i + 1] != ' ') 
0

Suggestion:

I would do an extension class for your string handling :

public static class StringExtensions
{
    public static string AllWordsInStringToUpperCase(this string value)
    {
        return string.Join(' ', value.Split(' ').Select(s => s.FirstCharToUpperCase()));
    }

    public static string FirstCharToUpperCase(this string word)
    {
        if (word.Length == 1)
            return word;

        return word.First().ToString().ToUpper() + word.Substring(1);
    }
}

Than use it like this for en example in an Console application:

class Program
{
    static void Main(string[] args)
    {
        Console.WriteLine("fuu a bar".AllWordsInStringToUpperCase());
    }
}

Output: Fuu a Bar

By that you can write some test so you know you get the behavior you want.

Peace!

Andreas Gustafsson
  • 321
  • 3
  • 5
  • 16
  • When I look at your code. Would it be possible for FirstCharToUpperCase to return IEnumerable as `yield return word.First().ToString().ToUpper(); yield return word.Skip(1);`? – Ondřej Kubíček Feb 02 '19 at 19:07