1

I tried to solve this problem in some test, but later when i ran it at home, it gave unexpected answer. I am not able to understand this code:

#include <stdio.h>
#include <conio.h>
#define swap(a,b) temp=a; a=b; b=temp;
int main()
{
int i, j, temp;
i=5;
j=10;
temp=0;
if( i > j) //evaluates to false
swap( i, j );
printf( "%d %d %d", i, j, temp); //expected output: 5 10 0
getch();
return 0;
}

Output i am getting is: 10 0 0

Please someone explain how is it working.

Mohit Jain
  • 30,259
  • 8
  • 73
  • 100
Roshan
  • 645
  • 1
  • 11
  • 36

2 Answers2

11

Code below

if( i > j) //evaluates to false
swap( i, j );

Becomes

if( i > j) //evaluates to false
temp=i; i=j; j=temp;

which is equivalent to

if( i > j) //evaluates to false
{temp=i;} i=j; j=temp;

If condition is false, there would be unexpected results as below

i=5;
j=10;
temp=0;
i=j;  /* i becomes 10 */
j=temp; /* j becomes 0 */

Learnings

  1. Try to put blocks (if, else, for, do, while) inside {}
  2. Avoid macros, prefer functions
  3. If you must use macros, use safe macros, for ex:

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

Note that there is no terminating semicolon after while(0)

Community
  • 1
  • 1
Mohit Jain
  • 30,259
  • 8
  • 73
  • 100
  • Thanx.. I got the missing link. :) – Roshan Sep 12 '14 at 05:22
  • 1
    Now that you see an example of macro horror, work out what this expands to: `swap(a[i++], b[i++])` – Jerry101 Sep 12 '14 at 05:51
  • 2
    @Jerry101 That line would yield undefined behavior no matter if swap is a macro or a function... – Lundin Sep 12 '14 at 06:30
  • @Lundin Possibly he meant `swap(a[i++], b[j++])` and ended up in a typo – Mohit Jain Sep 12 '14 at 06:30
  • @Lundin is right about undefined behavior. (Side point: In Java the order of evaluation is well defined.) Using Mohit's variation, a template function with T& parameters would do a useful swap, while a macro is bad. – Jerry101 Sep 12 '14 at 20:53
3

Expanding the macro, you get:

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

This is why seasoned c-programmers wrap macro bodies in do{...}while(0).

#define swap(a, b) do{temp=a; a=b; b=temp;}while(0)
Paul Hankin
  • 54,811
  • 11
  • 92
  • 118