Swapping algorithm giving everything the same value

Question

I have been given a task for my course to write a program which creates 52 card objects which each card has a value and a suit. After we create an array of the objects we have to switch the position of two cards at random. The problem is when I go to swap my cards that they both turn out to be the same. Here's the code that causes the problem:

void SwapCards(Card* cardOne, Card* cardTwo) {
    //swap cards here
    Card *temp = cardOne;
    *cardOne = *cardTwo;
    *cardTwo = *temp;
    //once it gets here all three variables end up with the same object?
}

Now here's the for loop that calls this function:

for (int i = 0; i < 104; i++) { 
    //get two random values and send them to SwapCards to switch both objects
    int c_one = RandomRange(0, 51);
    int c_two = RandomRange(0, 51);
    SwapCards(deck[c_one], deck[c_two]);
}

Any help with this will be greatly appreciated. I have spent a lot of time trying to figure it out but it just confuses me greatly.

Answer 1

"The problem is when I go to swap my cards that they both turn out to be the same."

You are loosing the current value of cardOne here:

*cardOne = *cardTwo;

Since temp still points to the same address as cardOne, cardOne's original value was not saved with it.

Change your code as follows, to save cardOne's value:

Card temp = *cardOne;
*cardOne = *cardTwo;
*cardTwo = temp;

---

The even better (and probably clearer) solution would be to use references instead of pointers:

void SwapCards(Card& cardOne, Card& cardTwo) {
    //swap cards here
    Card temp = cardOne;
    cardOne = cardTwo;
    cardTwo = temp;
}

And as a side note: This is what std::swap() would already do, no need to roll your own implementation here.

Answer 2

Remove the * from temp in Card *temp = cardOne;. Instead use

Card temp = *cardOne;

then

*cardTwo = temp;