CODEWITHSUNDEEP
c
Megamind
Megamind

Reputation: 41

Swap function in Function-Like Macros

Go through the following C code

# define swap(a,b) temp=a; a=b; b=temp;
main( )
{
    int i, j, temp;
    i=5;
    j=10;
    temp=0;
    if( i > j)
        swap ( i, j );
    printf ( "%d %d %d", i, j, temp);
}

Compiler Output:

10 0 0

I am Expecting this output

10 5 0

Why am I wrong??

Upvotes: 2

Views: 652

Answers (6)

JanK
JanK

Reputation: 160

Your problem is the if condition in this line:

if( i > j)
swap ( i, j );

After the preprocessor has replaced your define, these lines look like the following:

if( i > j)
temp=a; a=b; b=temp;

What you expect is the following:

if( i > j)
{temp=a; a=b; b=temp;}

which can be achieved by changing your define to 

#define swap(a,b) {temp=a; a=b; b=temp;}

EDIT: As mentioned by others, a more robust solution would be to define the macro as

#define swap(a,b) do{temp=a; a=b; b=temp;}while(1)

This still requires the variable temp to be declared by hand.

A good example of a type independent swap macro can be found here: https://stackoverflow.com/a/3982430/5237890

Upvotes: 2

FredK
FredK

Reputation: 4084

Another problem is the line

if( i > j)

In your case, i=5 and j=10, so the first statement of theswap() line is not executed. As @Kninnug pointed out (almost), without curly braces your code is actually

if(i > j)
    temp = i;
i = j
j = temp;

Now, since I=5 and j=10, only the last two lines are executed, so I becomes 10 (the value of j) and j becomes zero (the value of temp).

Upvotes: 1

Basile Starynkevitch
Basile Starynkevitch

Reputation: 1

It is much better to define statement-like macros with a do{...}while(0) like (see this):

#define swap(a,b) do { int temp=a; a=b; b=temp; } while(0)

since the do{....}while(0) construct is correctly understood (even as the then or else part of a conditional, etc. etc... as explained in other replies.

Actually, you'll still have issues if you invoke swap with a temp argument (e.g. swap(temp,xx)) or if you use it with a side effecting second argument (e.g. swap(x,y++) would increment y twice and not do what you want).

A pedantically robust swap might use concatenation at preprocessing level and some unique counter like the GCC specific __COUNTER__ predefined macro to generate a unique symbol (instead of temp)

Upvotes: 5

Dsel
Dsel

Reputation: 1037

When using a function-like macro the compiler just replaces the text in the code where you call the function with the text in your macro declaration. You did not use curly braces after the if statement which would be fine if you only needed to execute one line of code. But when the compiler does the text replacement for your macro function, there are now three lines of code that need to be nested under the if statement.

Adding curly braces around the line Swap ( a, b ) will fix your problem. Alternatively you can place curly braces around the three lines of code in your macro declaration.

Upvotes: 0

Paul Boutes
Paul Boutes

Reputation: 3315

You should add curly braces to your macro declaration to solve your problem.

Moreover, if you want to swap basic type, like int for example, you can use the XOR operator to swap the values without use an additional variable.

#define swap(a, b) { a ^= b; b ^= a; a ^= b; }

Upvotes: 2

Kninnug
Kninnug

Reputation: 8053

It's the lack of braces. This is one of the common pitfalls with macros. Let's see what happens:

if(i > j)
    swap(i, j);

becomes:

if(i > j)
    temp = a; a = b; b = temp;;

Made a little more readable:

if(i > j)
    temp = a;
a = b;
b = temp;

So the lines a = b; and b = temp; will always be executed, they fall outside the if body.

Either put braces around the if, or the macro.

Upvotes: 5

Related Questions