How would you improve upon this C code?

[Zilch]

A little something I quickly made.  I just got curious and wondered what different approach you might take when implementing this very simple program?

 

It counts the number of words by taking an input stream of characters and separating them into groups by a space, tab or newline/enter.

 

#include <stdio.h>

void main() {
	int c, state, final;

	final = 0;
	state = false;

	while ((c = getchar()) != EOF) {
		if (state == true && (c == ' ' || c == '\t' || c == '\n')) {
		final = final + 1;
		}
		else {
			if (c == ' ' || c == '\t') {
				state = false;
			}
			else {
				state = true;
			}
		}
		if (c == '\n')
			break;
	}
	printf("You have entered %d word(s)\n", final);
}

 

[Doominator2]

Just by quickly looking over it, I'd say the thing to improve the most on would be the readability of the code.

 

Just for readability, I'd declare state and final on separate lines for one. I'd also instead of having 

int c, state, final;

Also, set your undeclared values to NULL if your not declaring them, just incase. Might save you a lot of time in the future.

 

Something like this:

int c = NULL; 
int state = NULL;
int final = NULL;

 

 

I'd also use a switch statement instead

if (state)
{
	switch(c)
	{
		case ' ':
		final+= 1;
		break;
		case '\t':
		final+= 1;
		break;
		case '\n':
		final+= 1;
	}

}

Just my personal preference, makes it more readable and nicer in my opinion. There's also probably some other segments of the code you could rewrite/rework just for better readability. 

[Arno]

That's a nice little programming exercise.

 

It looks like when words are seperated by two or more spaces, it will count as extra words. That might not be the intended behavior. Hence, I would add state = false; after final = final + 1;.

 

I would personally also try to use more descriptive names for the variables. The variable final represents numberOfWordsFound. The variable state represents something like previousCharacterIsPartOfWord. But I have to admit that finding the right name for a variable can be hard.

 

[Zilch]

10 hours ago, Arno said:

That's a nice little programming exercise.

 

It looks like when words are seperated by two or more spaces, it will count as extra words. That might not be the intended behavior. Hence, I would add state = false; after final = final + 1;.

 

I would personally also try to use more descriptive names for the variables. The variable final represents numberOfWordsFound. The variable state represents something like previousCharacterIsPartOfWord. But I have to admit that finding the right name for a variable can be hard.

 

 

Thanks, Arno!  You are completely correct with that bug, not sure how that flew by me.  Must've been the late hour.

 

I'll keep in mind the variable naming scheme you suggest.  It's a fair bit better than my solution.

 

@Doominator2 I am currently going through The C Programming Language (2nd Edition), and have not yet finished.

 

I also agree, it is somewhat difficult to read, and the lack of comments certainly does not help.

 

The main strategy is to just get something done, rather than focus on the small stuff.  I really would like to improve my skills, but I have always found myself stuck on tiny things.

[Gez]

12 hours ago, Doominator2 said:

I'd also use a switch statement instead
 

if (state)
{
	switch(c)
	{
		case ' ':
		final+= 1;
		break;
		case '\t':
		final+= 1;
		break;
		case '\n':
		final+= 1;
	}

}

Just my personal preference, makes it more readable and nicer in my opinion. There's also probably some other segments of the code you could rewrite/rework just for better readability. 

I think I'd condense that to something like this, personally:

if (state)
{
	switch(c)
	{
		// Increase count when encountering white space
		case ' ':
		case '\t':
		case '\n':
			final+= 1;
			break;
	}

}

Just a personal preference, but when various cases have the exact same treatment, I prefer to explicitly treat them as the same thing instead of duplicating code for each.

[Martin Howe]

16 hours ago, Doominator2 said:

Also, set your undeclared values to NULL if your not declaring them, just incase. Might save you a lot of time in the future.

 

Something like this:

int c = NULL; 
int state = NULL;
int final = NULL;

 

Surely that practice only applies to pointers? The zero for type int is, well, 0 :) Yes, NULL is normally defined as 0 but its meant to be the zero for pointer types, not scalars; if I saw this in a random C program, I'd immediately think of c, state, and final as pointers and wonder why they are declared as int.

[Doominator2]

22 minutes ago, Martin Howe said:

Surely that practice only applies to pointers? The zero for type int is, well, 0 :) Yes, NULL is normally defined as 0 but its meant to be the zero for pointer types, not scalars; if I saw this in a random C program, I'd immediately think of c, state, and final as pointers and wonder why they are declared as int.

Yeah, I'm just paranoid though lol so I always set my undeclared variables to NULL no matter the datatype. But yes, setting it to 0 always works just a good for an int in this case, however a value of NULL is equivalent to just assigning a blank slate to a variable, while 0 is still a 4 byte numeric value being assigned to said variable. 

 

For a program like this there shouldn't be any problem, but just from old habit I normally set my undeclared variables to NULL anyways.

 

But then again, programming is not my main specialty so there is always things I could learn and improve on. 

[Martin Howe]

Sure, we can all learn things. NULL is specifically meant to mean "no address", that is, "this pointer points at nothing", but in C is done only by defining it as the number 0; in better programming languages, the compiler would give an error assigning NULL to a non-pointer type; try it on an int in C#, for example. As for 0 being 4 bytes; actually 0 is just a constant in the text; it's just a number and will work whatever the size of int.

 

Some languages, such as C#, support 'nullable' data types; that is you can have an int with a separate flag paired under the hood with it by compiler 'magic' that says 'this contains a value or doesn't'; in such languages, assigning NULL to an int isn't legal, but assigning it to a 'nullable of int' is; however, there's the extra overhead of maintaining that flag, so nullable scalar types are used only when needed, not usually for counters and so on.

[Arno]

In C++11 the keyword nullptr is introduced, which is also recognised as "a pointer that points to nothing", but with the advantage that it cannot be accidently used as the number 0.