1

I write this code to switch two values without temp storage and it works well, but when I try to use the same style to do other things, it doesn't work as I expected.

                `     
 #include <iostream>
 using namespace std;

 int main(){
   int i=100,j=200;
   //expect swich i and j
   i=j+i-(j=i);
   cout<<i<<"   "<<j<<endl;

   //expect j=100 and i=300 
   i=j+(j=i);
   cout<<i<<"   "<<j<<endl;
 }

result is:

200    100
400    200

so why doesn't the second (j=i) work?

can we just have a discussion about this situation? no one will maintain such code, including me.

Antonio Pérez
  • 6,702
  • 4
  • 36
  • 61
dongjk
  • 343
  • 1
  • 4
  • 15
  • 4
    Such code should not be written in any language. Think about maintainability - who is going to understand this in 3 months? You will probably need a few minutes yourself in a few weeks to find out what you were actually doing there. – Jan Thomä Nov 25 '11 at 06:57
  • 1
    Off topic comment. Code like that is a maintenance nightmare when you look at it a few weeks from when you first wrote it. I'd chose something more idiomatic. Also, your solution might flake when the values of `i` and `j` are close to the maximum permitted values. – Noufal Ibrahim Nov 25 '11 at 06:57
  • This is just for fun right? You are planning to use this for real? – David Heffernan Nov 25 '11 at 06:57
  • 1
    If you really want to do this safely, maybe look at [XOR swap](http://en.wikipedia.org/wiki/XOR_swap_algorithm) – juanchopanza Nov 25 '11 at 07:06
  • possible duplicate of [Undefined Behavior and Sequence Points](http://stackoverflow.com/questions/4176328/undefined-behavior-and-sequence-points). Specifically `j` is modified and also accessed other than to determine the new value of `j`. – Steve Jessop Nov 25 '11 at 09:41
  • Also, C++11 changes the way this is defined. In C++11 terminology, the two operands of built-in `+` are unsequenced, and it's UB to invoke a side-effect on `j` (assignment) that is unsequenced relative to a use of its value. – Steve Jessop Nov 25 '11 at 09:56

6 Answers6

8

This is undefined behaviour (UB). The operands to + can be evaluated in any order. The intent of your code relies on one particular order being chosen but your compiler is choosing the other evaluation order. Unlucky for you.

This experience should be enough to persuade you to avoid writing code like this. As soon as you start causing side effects in expressions you must be careful to avoid UB. Don't be afraid of using a local temp variable when swapping. And be even less afraid of using the swap routine built into the standard library.

One final point. The fact that you don't declare a variable does not mean that the generated code will not make use of a variable. It may be in a register, but so what, where do you think a compiler is going to put your temp int in a swap function?

David Heffernan
  • 601,492
  • 42
  • 1,072
  • 1,490
5

You could use the XOR swap :

i ^= j; j ^= i; i ^= j;

It's false economy on today's processors however.

Brett Hale
  • 21,653
  • 2
  • 61
  • 90
  • Could you explain why it won't work on today's processors? I ran it in python and it did work. – the_drow Nov 25 '11 at 07:23
  • @the_drow it does work. Answers does not say it does not work. Answer just says that there is no performance reason for using xor swap. – David Heffernan Nov 25 '11 at 07:25
  • 1
    @DavidHefernan: "It's false economy on today's processors however." I understood it from this sentence. Sorry, not a native English speaker. – the_drow Nov 25 '11 at 07:26
  • 1
    @the_drow: "a false economy" is an idiom, meaning that it appears to be economical (in this case it appears to save a variable), but in fact loses as much or more than it saves (in this case because with today's processors and compilers, there's no great likelihood of it resulting in faster code). – Steve Jessop Nov 25 '11 at 09:32
  • @SteveJessop Can you explain what it would lose of using XOR swap? In my understanding, it's a little slower than the one with temporary variables but it may save a few spaces. – lastland Feb 26 '12 at 09:32
4

As you give it your "method" has undefined behavior.

Between the previous and next sequence point an object shall have its stored value modified at most once by the evaluation of an expression. Furthermore, the prior value shall be read only to determine the value to be stored.

Practically speaken, you don't have any guarantee that the evaluation of y gives you the old value or the new value. A compiler could chose any of those.

Theoretically undefined behavior means that your code might eat your harddrive or spend all your money.

All this for a doubtful optimization that makes your code unreadable. Don't do cruft like that.

Jens Gustedt
  • 76,821
  • 6
  • 102
  • 177
2

Probably, your assumption about order of evaluation is wrong.

i=j+(j=i);

IIRC, it is not specified (for C++ I'm confident about this, see C++ standard §1.9 (15)).

If (j=i) is evaluated before j (after the equals sign) is evaluated, the result will be different than if you do it vice versa.

In short, please use std::swap() if you're using C++. This will clearly state your intent (and will allow implementers of the standard library to use optimizations).

Andre
  • 1,577
  • 1
  • 13
  • 25
1

try something like this :

a = a+b;

b = a-b;

a = a-b;

gprathour
  • 14,813
  • 5
  • 66
  • 90
1

With arithmetic operations you can get overflow. There is bits swap algorithm also:

a = a^b
b = a^b
a = a^b
Eugen Martynov
  • 19,888
  • 10
  • 61
  • 114