Using a function to find the average for number of letters and words

Question

This is my code:

#include <stdio.h>
#include <ctype.h>
int average_of_let_wor();
int main(void)
{
 double answer;
 answer = average_of_let_wor();
 printf("%.2lf", answer);
 return 0;
}
int average_of_let_wor()
{
 double numberOfLetters = 0;
 double numberOfWords = 0;
 int userInput;
 double answer;
 printf("please enter your input:\n");
 while ((userInput = getchar()) != EOF)
 {
 if (userInput == ' ' || userInput == '\t' || userInput == '\n')
 {
 numberOfWords++;
 continue;
 }
 else
 numberOfLetters++;
 }
 answer = numberOfLetters/numberOfWords;
 return answer;
}

• hjpotter92: And your question is?
• MNY: Hi, my question was is if this us a proper C code? Thanks for the reply! @BackinaFlash
• hjpotter92: Why are you storing an integer value to a double type variable? answer = average_of_let_wor();
• Loki Astari: @BackinaFlash: Welcome to "CodeReview". This is a review site. People post code to be reviewed and get tips on improving the code. If you think there is something worth changing the you should post an answer and explain why the change makes the code better (in any definition of better that you like).

Answer 1

First of all, be careful with types: to compute an average something, you will need a floating point number. However, the number of letters and words are countable and can therefore use the type unsigned int which are generally faster than floating point numbers.

Moreover, to detect a "space" character, you can use the standard function isspace from the header ctype.h.

Also, the variable answer is useless. You can get rid of it and simply return the answer the same line you're computing it.

Therefore, your function average_of_let_wor can be reduced to:

double average_of_let_wor()
{
 unsigned int numberOfLetters = 0;
 unsigned int numberOfWords = 0;
 int userInput;
 printf("please enter your input:\n");
 while ((userInput = getchar()) != EOF)
 {
 if (isspace(userInput))
 {
 numberOfWords++;
 }
 else
 {
 numberOfLetters++;
 }
 }
 return (double)numberOfLetters / (double)numberOfWords;
}

• MNY: Quick question - this function obviously work perfectly. Something i wanted to understand: the first if block with the isspac() function...lets say my input is 3 times enter(return), isn't numberOfWords suppose to be 3..? again, it works perfectly and this scenario leaves numberOfWords still as 0, but how come? @Morwenn
• Morwenn: Reading newline characters is always a problem. I don't know all the subtleties of the mechanism. You should ask the question on stackoverflow.com or check whether the question xas already asked somewhere else.
• William Morris: why use an unsigned type?
• Morwenn: @WilliamMorris A number of letters/words can not be negative. The standard type to ensure this should be size_t btw.
• Loki Astari: Have issues with a lot of points: 1) I agree that unsigned int is a better choice for counting, but Int's faster than float? I doubt that is true in any measurable way. Even if it was who cares. 2) Though I agree about the use of isspace() it is not as simple as that. You need to explain that it changes the behavior of the code (more characters are space). 3) the variable answer is useless: Not true it makes the self documenting. It also helps when debugging as you can examine the result via the variable rather than manually evaluating the expression. Overall a bad attempt at a review.

Answer 2

Maintainability is an important part of coding.
I see where you are going with the name of the function. But if you go 80% there you may as well go the whole way. Name the function for what it does (don't chop the words).

int average_of_let_wor();
// Would have preferred
int average_of_letters_and_words();

Are you really returning an integer from the function? By your use case above you assign the result to a double. But if you return an int you will truncate the value. ie. you will get 2 rather than 2.5 etc.

Here you assign to double. So I suspect you want to actually return a double.

 double answer;
 answer = average_of_let_wor();

As pointed out by @Morwenn. Counts are usually integer. Personally I would also make them integer (but this will cause a slight problem below when you calculate the answer (as integer division returns an int so at that point you must coerce the values back to a double)). Thus this is really a 50/50 coin flip on which is the better technique and leaving them as double is quite acceptable.

 double numberOfLetters = 0;
 double numberOfWords = 0;

The easy way to check for space is isspace().

 if (userInput == ' ' || userInput == '\t' || userInput == '\n')

But these are not the only space characters so be careful. If your specs say only to test for these three then using isspace() may give you different results.

You are forgetting to test if the last character was a space. Thus words separated by double space count as two words (or more (if you use more spaces)). Also do you need to take into account punctuation and other characters?

 {
 numberOfWords++;
 continue;
 }

Its a good habit to use braces on all nested code blocks.
There are some corner cases where you may accidentally modify the code and a subsequent statement is not inside the else branch (and it is hard to spot (say inside a macro). So for maintainability please get into the habit of using {}

 numberOfLetters++;

Here I totally disagree with Morwenn.

 answer = numberOfLetters/numberOfWords;
 return answer;

There is nothing wrong with assigning the result to answer. It will make absolutely no difference to the resulting code that is generated and it helps document the code by explaining what you are doing here.

Another added advantage is that when you are debugging, you can hover over the variable answer and see that it has been correctly defined.

• tb-: Two small points. For the function name I would suggest average_letters_per_word(). About the answer variable: I do not agree with the arguments. The return clearly indicates and documents, that this is a result/answer. If necessary, the debugger can catch the return value from the function. I agree, that this does not make any difference to the resulting code, if any compiler optimization is done.