Small C issue.. "expected expression before.."

skuzye · 12-29-2008, 11:28 AM

Note: The main questions has been solved already but there's good info for novices down there. If you have some spare time, it won't hurt to read further

Hello, I'm currently studying C with "C Programming Language Second Ed." and I just can't figure out what's wrong with my code.

I'd appreciate a small hint here.

The error:

Quote:

supercount2.c: In function 'main':
supercount2.c:23: error: expected expression before 'else'

note: I marked the line which contains the else referred.

And the code:

Code:

#include <stdio.h>

main() {
// This program counts spaces (blanks), characters, tabs, newlines and words.

#define IN 1;	// defines if it's in a word
#define OUT 0;	// or not ;)

	long int blanks = 0, chars = 0, tabs = 0, newlines = 0, words = 0;
	int c;
	char state;
	
	while ((c = getchar()) != EOF) {
		++chars;
		if (c == ' ') 
			++blanks;
		if (c == '\t')
			++tabs;
		if (c == '\n')
			++newlines;
		if (c == ' ' || c == '\t' || c == '\n')
			state = OUT;
		else if (state == OUT) { // error before this line
			state = IN;
			++words;
		}
	}
	
	printf("Blanks: %ld\nCharacters: %ld\nTabs: %ld\nNewlines: %ld\nWords: %ld\n", blanks, chars, tabs, newlines, words);

	return 1;
}

Thanks. Skuzye

Guttorm · 12-29-2008, 11:36 AM

Code:

#define IN 1;	// defines if it's in a word
#define OUT 0;	// or not ;)

Don't use ; in #defines. Also, the // will be included in the value, so you will get comments in funny places.

Code:

#define IN 1	/* defines if it's in a word */
#define OUT 0	/* or not ;) */

chrism01 · 12-29-2008, 06:07 PM

'else' can only come after a matching 'if', ie

Code:

if (x == 'x' )
{
    something;
}
else
{
    something_else;
}

You're using the single line 'if'

Code:

if( x == 'x')
    something;

no curly parentheses after the 'if' means a single line 'if' block.
Personally I ALWAYS use parentheses to avoid probs like that, also if you go to add a debug line to the (implied) if block, it'll fail...

Its also unusual to have #define other than at the top of the prog, just after the #includes. Its just a directive to the pre-processor to do a simple text substitution everywhere it matches, as per Guttorm's comment.

skuzye · 12-29-2008, 06:27 PM

Edit: It's interesting because on the book it's written without curly braces and it's supposed to work but it actually doesn't.

Thanks a lot guys... Skuzye

swodniw · 12-29-2008, 06:46 PM

Quote:

Originally Posted by chrism01

'else' can only come after a matching 'if', ie

Code:

if (x == 'x' )
{
    something;
}
else
{
    something_else;
}

You're using the single line 'if'

Code:

if( x == 'x')
    something;

no curly parentheses after the 'if' means a single line 'if' block.

That I am sorry to say is utter nonsense. Just try looking at any examples in the standard.

skuzye · 12-29-2008, 08:23 PM

Quote:

Originally Posted by swodniw

That I am sorry to say is utter nonsense. Just try looking at any examples in the standard.

You're right, after fixing #define issue I tried to compile without curly braces and it worked flawlessly.

johnsfine · 12-30-2008, 08:59 AM

Guttorm and swodniw's answers were correct (at least the main part) and maybe enough. But I think Tinkster might still be missing the key point:

Code:

if( x == 'x')
    something;
else
    ...

is OK

Code:

if( x == 'x')
    something;;

is OK (with an extra ; and without an else).

Code:

if( x == 'x')
    something;;
else
    ...

is not OK. With an extra ; the else doesn't work.

If the ; is included when you define OUT and included again when you use OUT then the compiler must parse two ; in a row.

Quote:

Originally Posted by Guttorm

Also, the // will be included in the value

Really? I'm not sure about it and I don't have time to test at the moment, but I think I've used // comments on #define instructions many times assuming the // and comment did not become part of the definition. I never had any problems with that.

taylor_venable · 12-30-2008, 09:30 AM

Quote:

Originally Posted by johnsfine

Really? I'm not sure about it and I don't have time to test at the moment, but I think I've used // comments on #define instructions many times assuming the // and comment did not become part of the definition. I never had any problems with that.

With GCC, it depends on what standard you're compiling against. With -std=c89 you will get the // ... included, but with -std=c99 you won't. The default in my version (3.3.5) uses (I believe) -std=gnu89 by default which allows C++ line comments and thus eliminates them without putting them in the definition.

skuzye · 12-31-2008, 07:46 AM

I was examining again this code and I noted that I stored an integer number in a char type variable and I haven't noted any issues with that. Is it ok? I mean, I can see it's possible but is it a good practice?

I just can't see why to use an integer(4bytes) or even a short integer(2 bytes) if I just need to store 1 or 0 and a char type is 1 byte.

Skuzye

johnsfine · 12-31-2008, 08:34 AM

Quote:

Originally Posted by skuzye

I was examining again this code and I noted that I stored an integer number in a char type variable

Do you mean

Code:

state = OUT;

Quote:

and I haven't noted any issues with that. Is it ok?

It is ok.

Quote:

I mean, I can see it's possible but is it a good practice?

On most architectures you should assume that int is the fastest data type for function local variables and that for an individual (not array) function local variable, the space advantage (if it exists at all) of using something smaller than int is insignificant.

Depending on what else you do with the variable "state", this is likely to be one of the less common cases (for x86 or x86-64) where a char results in faster code than an int.

I personally would tend to use a char for that variable, but I consider that choice pretty arbitrary. Neither char nor int would be considered more or less "good practice" in this case.

Best practice in C++ for that variable would be either:
Rename it so that it is meaningful with values true and false, and then use a bool.
or Define IN and OUT within an enum (much better than #define anyway) and then make "state" that enum type.
(but I forget how much of the above is reasonable in C rather than C++).

Quote:

I just can't see why to use an integer(4bytes) or even a short integer(2 bytes) if I just need to store 1 or 0 and a char type is 1 byte.

Saving storage space should not be considered AT ALL for a scalar local variable. So I don't object to "state" being a char, but I do strongly object to that reason for "state" being char.

swodniw · 12-31-2008, 09:42 AM

johnsfine has pointed out the why's and why not of using a char/int; yet as you are using C and there are only two possible values have you considered using a bool. bool was introduced to the language with C99 and requires the header stdbool.h
You could change the code to something like

Code:

bool in = false;
/*removed code here*/
else if (in) {
in = !in;
++words;
}

johnsfine · 12-31-2008, 10:10 AM

Swodniw, I think you have that test backwards and I think

Code:

in = !in;

is pointlessly confusing. I also think the name "in" is not clear enough for a boolean state variable.

Skuzye, I didn't notice before that you failed to initialize "state". That is another bug in your code.

Using bool (assuming that C++ or C99 construct is OK to use) I suggest:

Code:

	bool in_word = false;
        ...
		if (c == ' ' || c == '\t' || c == '\n')
			in_word = false;
		else if ( ! in_word ) {
			in_word = true;
			++words;
		}

swodniw · 12-31-2008, 10:17 AM

Quote:

Originally Posted by johnsfine

Swodniw, I think you have that test backwards and I think

Code:

in = !in;

is pointlessly confusing. I also think the name "in" is not clear enough for a boolean state variable.

It would seem I do, yet the code was just for demo and it is personal taste if in=!in is confusing. A better name would be whitespace.

Quote:

Skuzye, I didn't notice before that you failed to initialize "state". That is another bug in your code.

quite correct.

skuzye · 12-31-2008, 10:45 AM

Quote:

Originally Posted by johnsfine

Do you mean

Code:

state = OUT;

Yes.

Quote:

Originally Posted by johnsfine

Saving storage space should not be considered AT ALL for a scalar local variable.

I know it's only a small thing, but can it make any difference in a big project (considering you use similar implementations a lot)?

Quote:

Originally Posted by johnsfine

So I don't object to "state" being a char, but I do strongly object to that reason for "state" being char.

I'm sorry, I didn't understand what you meant

Quote:

Originally Posted by johnsfine

Rename it so that it is meaningful with values true and false, and then use a bool.

Quote:

Originally Posted by swodniw

...have you considered using a bool. bool was introduced to the language with C99 and requires the header stdbool.h

I thought about it but as I said I'm learning from C Programming Language and bool was not introduced to me yet :P (I don't see why, it seems so simple).

Skuzye

johnsfine · 12-31-2008, 11:30 AM

Quote:

Originally Posted by skuzye

I know it's only a small thing, but can it make any difference in a big project (considering you use similar implementations a lot)?

No it (the space of the local variable itself) can't make a difference.

The compiler probably would use registers in a way such that there isn't actually any difference in data size. If there were a difference in data size, any difference in code size would likely be bigger than the difference in data size. Unless you are very experienced in both asm and C, you can't predict the difference in code size that results from using char vs. int for a function local variable. More often than not the code for char will be larger (and slightly slower) than the code for int. But it can go either way.

Quote:

I'm sorry, I didn't understand what you meant

I meant that this data type decision should be made on the basis of coding style or readability of the code or maintainability of the code or (if it matters) execution speed. The decision should not be made on the basis of data size. When you have a big array (or more complex container) of objects, you should consider data size as a factor in designing those objects. When you have ordinary objects one at a time, you don't consider their size.

Quote:

I thought about it but as I said I'm learning from C Programming Language and bool was not introduced to me yet

How about "enum"? Was that introduced yet? I think enum is the best data type for this in C.