vector elements suddenly changing?

[kingkush]

I have a vector of objects passed into a function but it suddenly changes when i try to use them. Once the vector is passed in I use a print statement to see the contents and its all correct. But once I start to copy the elements to another vector I'm getting the wrong output.

void passCards(int numPlayers, std::vector<CardTemplate> deckOfCards, Player p1, Player p2, Player p3, Player p4)
{
    //Distribute cards to number of Players
    switch(numPlayers)
    {
        case 2:
            //Test to check whats in deck
            for(int i = 0; i < deckOfCards.size(); i++)
            {
                std::cout << deckOfCards[i].suit << " : " << deckOfCards[i].cardNum << std::endl;
            }
            std::cout<<std::endl;
            std::cout<<std::endl;
            std::cout<<std::endl;
            
            for(int i = 0; i < deckOfCards.size() - 1; i++)
            {
                p1.playerHand.push_back(deckOfCards[i]);
                //LOOK HERE
                std::cout << p1.playerHand[i].suit << " : " << p1.playerHand[i].cardNum << std::endl;
                i++;
                p2.playerHand.push_back(deckOfCards[i]);
                //LOOK HERE TOO
                std::cout << "2----" << p2.playerHand[i].suit << " : " << p2.playerHand[i].cardNum << std::endl;
                i++;
                //Will not be used just to keep cards randomized for 2 players (Keep to let users select deck?)
                //playerThree.push_back(deckOfCards[i]);
            }
            break;

The output of the deck's first print:

Diamond : 3
Diamond : 4
Diamond : 5
Diamond : 6
Diamond : 7
Diamond : 8
Diamond : 9
Diamond : 10
Diamond : 11
Diamond : 12
Diamond : 13
Diamond : 14
Diamond : 15
Clover : 3
Clover : 4
Clover : 5
Clover : 6
Clover : 7
Clover : 8
Clover : 9
Clover : 10
Clover : 11
Clover : 12
Clover : 13
Clover : 14
Clover : 15
Heart : 3
Heart : 4
Heart : 5
Heart : 6
Heart : 7
Heart : 8
Heart : 9
Heart : 10
Heart : 11
Heart : 12
Heart : 13
Heart : 14
Heart : 15
Spade : 3
Spade : 4
Spade : 5
Spade : 6
Spade : 7
Spade : 8
Spade : 9
Spade : 10
Spade : 11
Spade : 12
Spade : 13
Spade : 14
Spade : 15

Output once copied to another vector:

Diamond : 3
2---- : 0
 : 0
2----Diamond : 3
 : 0
2---- : 0
 : 0
2---- : 0
 : 0
2----Diamond : 8
 : 0
2----Diamond : 11
 : 0
2----Diamond : 14
 : 0
2----Clover : 4
 : 0
2---- : 0
 : 0
2---- : 0
 : 0
2---- : 0
 : 0
2---- : 0
 : 0
2---- : 0
 : 0
2---- : 0
 : 0
2---- : 0
 : 0
2---- : 0
 : 0
2---- : 0

[Peter87]

I don't think you should be incrementing i on line 21 and 25.

[kingkush]

I'm passing cards to different players from a deck of cards. I increment the i so the next player gets the next card in the deck.

Do you know what else could possibly cause this? I was also getting weird output with random code snippets along with numbers being printed out when i passed the players in my reference. (im guessing maybe memory addresses or something)

[keineahnung]

Hi.

Inside your for loop:

- i = 0
- card #0 dealt to player 1
- increment iterator
- i = 1
- card #1 dealt to player 2
- increment iterator
- i = 2

back to top of for loop
- increment iterator
- i = 3
- card #3 dealt to player 1
...

Card #2 is never dealt.
I agree with Peter87 - at least one increment too many inside the for loop.

[kingkush]

I'm skipping 1 card each turn. Its a 52 card deck and I'm giving each player 17 cards. Essentially the skipping should represent a third deck being passed to. The function incrementation was working fine for what i needed to do but that still doesn't explain why the vector elements are changing

[keineahnung]

Fair enough.
Just a thought - you're passing Player objects in by value.

void passCards(int numPlayers, std::vector<CardTemplate> deckOfCards, Player p1, Player p2, Player p3, Player p4)
Those copies will expire when the function returns.

This is a very uncommon design. Would it fix some things if you passed by reference? I.e.:
void passCards(int numPlayers, std::vector<CardTemplate> deckOfCards, Player& p1, Player& p2, Player& p3, Player& p4)

[keineahnung]

Actually another thing.

Look at how you're printing out the values:

p1.playerHand.push_back(deckOfCards[i]);
                //LOOK HERE
                std::cout << p1.playerHand[i].suit << " : " << p1.playerHand[i].cardNum << std::endl;
                i++;
                p2.playerHand.push_back(deckOfCards[i]);
                //LOOK HERE TOO
                std::cout << "2----" << p2.playerHand[i].suit << " : " << p2.playerHand[i].cardNum << std::endl;

The first bit in bold. Assuming that we're in the first iteration of the loop and that p2.playerHand is an empty vector. You push_back card #1.
Now the second bit in bold. You access element #1 of p2.playerHand to print out the suit. But there won't be anything in slot #1 yet - what you mean to do is access element #0 but your iterator's now out of sync with the for loop (because you manipulate it inside the for loop).
Next time round you'll be even further off - there'll be 2 elements in each playerHand but you'll be trying to access the values for elements #3 and #4. Etc.
You'd need a separate counter to keep track of how many cards you've dealt so you can access the right element.
Hope that helps.

[kingkush]

@keineahnung

THANK YOU SOOOOOOOO MUCH!! For the love of god I could not figure it out. That was why it was printing all weird lol.

Now im thinking this function may have been working fine but the bug is somewhere else in my code...

[kingkush]

@keineahnung

Your previous post solved my bug. I was just wondering what the difference when passing the func(object) vs func(&object)? Don't they both use a pointer to the object?

[keineahnung]

@kingkush
No - one makes a copy and the other passes a reference. See this example (a bit messy but it illustrates the point).

#include <iostream>

class A
{
    public:
        A(int x) : myx(x) {}
        int getmyx() { return myx; }
        void setmyx(int x) { myx = x; }
    private:
        int myx;
}; // class A


// Parameter is a copy of object.  There'll be no change in calling
// environment.
void bar(A theA)
{
    theA.setmyx(17);
} // bar

// Parameter is a reference - changes will affect calling environment.
void foo(A& theA)
{
    theA.setmyx(17);

} // foo


int main()
{
    A someA(4);
    std::cout << someA.getmyx() << "\n";
    someA.setmyx(8);
    std::cout << someA.getmyx() << "\n";

    std::cout << "call bar()\n";
    bar(someA);
    std::cout << someA.getmyx() << "\n";

    std::cout << "call foo()\n";
    foo(someA);
    std::cout << someA.getmyx() << "\n";

    return 0;

} // main 

If you want to make sure a function can't alter an object that's passed in by reference (and document to an end user that this won't happen), ask your compiler to enforce this by making that parameter const - as an example, try changing the function signature of foo() to:
void foo(const A& theA). Your compiler should reject it.

In more idiomatic C++, your function header would maybe look like this (- note I make the vector a const ref):
void passCards(int numPlayers, const std::vector<CardTemplate>& deckOfCards, Player& p1, Player& p2, Player& p3, Player& p4)

[kingkush]

Thanks for your explanation! I mustve gotten confused with what my professor said. I sometimes get confused when i should use & lol but thanks for helping clarify!