Console application that counts the number of words in a sentence. (C#)

Question

This program checks every character in a sentence. Every time the character is a space(" ") the numberOfWords (variable) will be incremented by 1. Is this the right way to do it?

        string sentence;
        int numberOfWords;
        int sentenceLength;
        int counter;
        string letter;


        Console.Write("Sentence :");
        sentence = Console.ReadLine();
        sentenceLength = sentence.Length;
        numberOfWords = 1;
        counter = 0;

        while (counter < sentenceLength)
        {
            letter = Convert.ToString(sentence[counter]);
            if (letter == " ")
            {
                numberOfWords++;
                counter++;
            }
            else
            {
                counter++;
            }

        }

        Console.Write("Number of words in this sentence :");
        Console.WriteLine(numberOfWords);
        Console.ReadLine();

this is right if it's guaranteed that there's only 1 space between words — merkithuseyin, Oct 06 '18 at 10:52
Not really, this is pretty bug riddled; double spaces as previous comment points out, leading spaces, trailing spaces, empty sentences, etc. ... there are tons of scenarios where your code will give wrong results... — InBetween, Oct 06 '18 at 10:55
Also, converting a `char` to `string` using `Convert` is pretty yucky. Simply compare the `char` to `' '`, or better yet, use the suitably named `char.IsWhiteSpace(sentence[counter])` to do the job. — InBetween, Oct 06 '18 at 10:57
https://codereview.stackexchange.com/ should be used for Code Review — Roman Pokrovskij, Oct 06 '18 at 17:30
What if the sentence length is 0? You're going to report one word, but there aren't any words. — Jim Mischel, Oct 06 '18 at 19:50

InBetween · Answer 1 · 2018-10-06T16:47:31.740

Well, the easy answer is; don't reinvent the wheel, use existing tools:

var numberOfWords = 
   sentence.Split(
       ' ',                            
       StringSplitOptions.
           RemoveEmptyEntries).Length;

But that would be cheating...

So, taking your code, there are a few things that need to be fixed:

First, don't make your method do too many things. There is no reason why a method counting words should know anything about how to output the result to any given user interface. Simply make a method that knows how to count words and returns the number of words:

public static int CountWords(string sentence) { ...}

Now you can reuse this method in any type of application; console, windows forms, WPF, etc.

Second, take corner or trivial cases out of the equation fast. Null sentences are either an error or have no words. Make a choice on how you want to process this scenario. If 0 words makes sense, you can solve a few cases in one strike:

    if (string.IsNullOrWhiteSpace(sentence))
        return 0;

Third, don't perform unnecessary conversions; converting chars to strings simply to perform an equality check with " " is wasteful. Compare chars directly (' '), or use the aptly named char.IsWhiteSpace(+) method.

Fourth, your logic is flawed. Double spaces, leading spaces, etc. will all give you wrong results. The reason being that your condition on when to count a word is faulty. Encountering a whitespace doesn't necessarily mean a new word is on the way; another whitespace might be waiting, you’ve already encountered a white space in the previous iteration, the sentence might end, etc.

In order to make your logic work you need to keep track of what happened before, what’s happening now and what will happen next... if that sounds messy and over complicated, don’t worry, you are absolutely right.

A simpler way is to shift your logic just a little; let’s say we encounter a new word everytime we find a non whitespace(*) that is preceded by a whitespace. What happens after is irrelevant, so we’ve just made things a lot easier:

var counter = 0;
var words = 0;,
var previousIsWhiteSpace = false;

while (counter < sentence.Length)
{
    if (char.IsWhiteSpace(sentence[counter]))
    {
        previousIsWhiteSpace = true;
    }
    else if (previousIsWhiteSpace) 
    {
         words += 1;
         previousIsWhiteSpace = false;
    }

    counter += 1;
}

Put it all together and you are done.

(+) this will actually flag more than a regular space as a valid whitespace; tab, new line, etc. will all return true.

(*) I’m ignoring scenarios involving punctuation marks, separators, etc.

score 0 · Answer 2 · answered Oct 06 '18 at 11:11

Just sticking with the style of your implementation, assuming the input is only split by single spaces, it's much nicer to just split your sentence string on each white space character.

string[] words = sentence.Trim().Split(null);

using null as the argument, white space will be used to split. Trim() removes trailing and leading space characters. Then, using words.Length you can easily get how many words there are separated by white space. However, this won't account for double spaces or empty sentences. Removing double or more spaces is best achieved with regex.

RegexOptions options = RegexOptions.None;
Regex regex = new Regex("[ ]{2,}", options);     
sentence = regex.Replace(sentence, " ");

`StringSplitOptions.RemoveEmptyEntries` takes care just fine of multiple consecutive whitespaces. No need to use reflex for such a trivial task. — InBetween, Oct 06 '18 at 14:16

Console application that counts the number of words in a sentence. (C#)

2 Answers2