Counting the number of words in a string using C

Question

I wrote a simple program to count the number of words in a string using C. Would you mind reading it and telling me if the method used is acceptable? Or if there are style errors or ways I can improve it?

 /*Procedure that counts the number of words in a string
    A word is any set of characters (or a single character)  separated
    from the word after it and before it by a space* 

    For example, "a" is one word, "!" is one word, "peut-etre" is one word, "Builder Jack" are two words*/

    #include <stdio.h>


    int word_count(char *array){
        int count = 0; /*Number of words*/

        while(*array != '\0'){
            if(*array == ' '){
                count++;
            }
            array++;
        }
        return (count+1);
    }

    int main(void){
        char line[1000];

        puts("Enter text");

        fgets(line, sizeof(line), stdin);

        printf("Word count is %i", word_count(line));

        return 0;
    }

Answer 1

As has already been said, your code is not very robust and whilst it will work for some happy path scenarios, there is a lot of scope for error.

Variable initialisation

You don't initialise line. Consequently it has an undefined value. In local testing this isn't going to be a huge problem because you call fgets and read from stdin into the variable. However, as indicated by @πάντα-ῥεῖ's example this isn't always the case. It's very possible for calls to fgets to fail in which case line will contain whatever it is initialised to.

Unless you're really sure you don't need it, err on the side of caution and initialise your variables:

char line[1000]="";

Error Checking

As I've said, fgets can fail. When it does, it returns NULL. You're not currently checking the fgets call's return value. In most cases when you're dealing with stdin, you shouldn't encounter errors so you can avoid the need to check the return value through correct variable initialisation. However, if you're not going to initialise your variables it's a good idea to check if fgets returns a value to indicate that it has successfully read something.

End of line

fgets will read until the end of either the buffer size supplied or the end of line, whichever comes first. If it reaches the end of the buffer (sizeof(line)), then the buffer will be NULL terminated. If on the other hand it reads to the end of line, the buffer will include an end of line character '\n' before the NULL termination. At the moment, you're treating this end of line character as part of a word which is probably incorrect.

What's a word?

Currently your method is actually returning the number of spaces in the input string, plus 1. This works for simple cases where you assume that you start with a word and any space will be followed by a new word. However, there are lots of edge cases where this isn't going to be true. I put a double space after a full stop, should that really count as two words? If I have nothing but spaces in the string I would expect 0, that's not what I get etc.

It may be that for your usage, your definition of a word is enough, however it doesn't feel like the right definition. It seems like groups of whitespace should be treated as a single word boundary, rather than always being treated as the start of a new word. You have decided that punctuation on its own is a word, does that really make sense?

Whitespace Vs ' '

Currently, you're only recognising actual space characters as word separators. This may be correct for you, however as suggested by @Incomputable you could use isspace to check for a wider range of whitespace characters. This would make your code more robust and able to deal with other whitespace characters like tabs.

Output considerately

puts automatically appends a newline to the end of the string it writes to stdout. This isn't the case for printf, you need to append it yourself. You don't currently append a new line, so it's messy for the user.

Naming & Comments

Comments should tell you add useful insight into what's going on in your code. The comment at the top of the file is good, it contains a description of your specification for a word. This is useful, although it's in the wrong place. It's a description of what the word_count method does, why isn't it directly above the method?

You also have this comment:

int count = 0; /*Number of words*/

If you give count a proper name, then you don't need the comment:

int wordCount = 0;

Indentation

Your file indentation seems a bit off. It may be a copy/paste error, however I'd expect method signatures to start at the extreme left of the line, not floating one indentation in. If it is a copy/paste error, in future, try to make sure your code in the question looks like it does in your editor.

Answer 2

• Since input string is not modified, it should be marked as const.
• int might be small to count the words, since theoretical maximum length of input string is maximum value of size_t. Hence, use size_t.
• No check for empty string or invalid input (e.g. NULL). In the latter case caller effectively invoked undefined behavior. The check probably won't affect the performance.
• No comments are better than useless comments
• Failed edge case test: all spaces. Will incorrectly return 1.
• You could use isspace() to make the code more robust.
• array doesn't sounds like a good name. text would be better.

Other things seems to be fine.