views:

271

answers:

4

I run my C code in vs2010 (win32 console application). It was compiled as c++ application.

#include "stdafx.h"

#define     YES     1;
#define     NO      0;

// function to determine if an integer is even
int isEven(int number)
{
    int answer;

    if ( number % 2 == 0)
        answer = YES;
    else
        answer = NO;    
    return answer;

}

int main()
{
    int isEven(int number);

    if (isEven(17) == YES)
        printf("yes "); 
    else
        printf("no ");


    if ( isEven(20) == YES)
        printf("yes\n");
    else
        printf("no\n");

     return 0;
}

Compiler error as below.

p300.cpp(18): error C2181: illegal else without matching if
p300.cpp(30): error C2143: syntax error : missing ')' before ';'
p300.cpp(30): error C2059: syntax error : ')'
p300.cpp(31): warning C4390: ';' : empty controlled statement found; is this the intent?
p300.cpp(33): error C2181: illegal else without matching if
p300.cpp(37): error C2143: syntax error : missing ')' before ';'
p300.cpp(37): error C2059: syntax error : ')'
p300.cpp(38): warning C4390: ';' : empty controlled statement found; is this the intent?
p300.cpp(40): error C2181: illegal else without matching if

Then I also tried to insert several of { } for each of if-else condition statement, but the code still compiled failed. What's wrong with my code? Thank you.

+6  A: 

The compile error is due to the semicolons on your #define statements. Remove them.

#define is a preprocessor macro, not c syntax. It doesn't need a semicolon. The preprocessor does straight substitution on YES and NO, which makes:

if ( number % 2 == 0)
    answer = YES;
else
    answer = NO;

Turn into:

if ( number % 2 == 0)
    answer = 1;;  // <-- Notice the two semicolons!
else
    answer = 0;;

That makes two statements between if and else, so compiler errors ensue. I suspect you get different compiler errors when you add { and } due to

if (isEven(17) == YES)

becoming

if (isEven(17) == 1;)

By the way, this question is tagged c, but your filename is .cpp, which is a common suffix for c++. If you are using c++, definitely use the bool type.

bool is_even = true;
bool is_odd = false;
Stephen
+1, This is the reason you should always enclose macro expressions in parenthesis (ie, #include YES (1) ) and why I always enclose if/while/for statement in brackets even if there is only a single statement in the clause
Akusete
+2  A: 

to start with, remove the semicolons from the end of the defines

#define     YES     1
#define     NO      0
Detmar
+1  A: 

There is nothing wrong with placing a semicolon in a #define statement. It simply becomes part of the macro. You defined

#define     YES     1;

So, your main function becomes like this:

int main()
{
    int isEven(int number);

    if (isEven(17) == 1;)
        printf("yes "); 
    else
        printf("no ");


    if ( isEven(20) == 1;)
        printf("yes\n");
    else
        printf("no\n");

     return 0;
}

While there is nothing wrong with placing a semicolon, in this case what you probably want is to remove the semicolon, and then the statement will become what you want.

Dumb Guy
While it is true that there is nothing wrong with placing a semicolon in a #define, IMNSHO it's seriously braindead to put a semicolon _at the end_ of a #define.
ninjalj
+2  A: 

As mentioned by other users, removing the semicolons should fix your problem at hand. However, I thought it might be handy to point out a few more things you should be aware of while writing macros.

Although handy at times, it is good to be aware of the pitfalls of using Macros in C.

1) There is no explicit type casting here.

i.e 'NO' can be a uint16_t or a uint32_t.

You defined your variable in the above example as an int. If we assume you are on a 16 bit machine, your variable answer is 16 bits long. However you have not defined a type for NO. In this case it isn't much of an issue, since the value of NO is small. However if you were comparing it against your 'answer' then it should be clear what the maximum value of answer should be. You would also perhaps encounter issues, if you tried to port this code to a 32-bit machine.

  • Type Casting will help you achieve a predictable outcome of your code, and also clearly indicate your intention while writing the code.

Good practices for defining the following:

#define     YES     1  
#define     NO      0

Explicit type casting:

#define     YES  (uint16_t)   1  
#define     NO   (uint16_t)   !(YES)  

Another way of telling the compiler to do what you want it to: If you had the number 0xFFFF. Depending on whether it is signed or unsigned, this could have different meaning to your compiler. So if your intention was for it to be unsigned, you can explicitly tell the compiler to treat it that way, by doing the following:

#define MY_LARGE_CONSTANT    0xFFFFU

Notice the 'U'. This tells the compiler it is unsigned.

I think these are good practices, that should be kept in mind while writing macros. It forces you to think about your intention for the constant's usage. Also habits like these developed early on, definitely go a long way in your career.

IntelliChick