std::iterator error!

I'm trying to make an iterator for the cards vector but it doesn't seem to be working correctly, here's the code:

Code:

---

        void Deal(int howmanycards, vector<Card> hand, vector<int>)
        {
                deck.cards::iterator it = deck.cards.begin();
                for (it; it = deck.cards[howmanycards]; it++)
                {
                        hand.push_back(deck.cards[it]);
                        deck.cards.erase(it);
                }
        }

---

I'm just trying to make a function that will deal cards to a hand, it takes an int for how many cards to draw, a card vector for where to put them, and an int vector to determine if they are face up or face down.

Here are the errors I get.

Code:

---

------ Build started: Project: Blackjack, Configuration: Debug Win32 ------
Compiling...
Blackjack.cpp
c:\visual studio 2005\projects\blackjack\blackjack\dealer.h(30) : error C2653: 'cards' : is not a class or namespace name
c:\visual studio 2005\projects\blackjack\blackjack\dealer.h(30) : error C2039: 'iterator' : is not a member of 'Deck'
        c:\visual studio 2005\projects\blackjack\blackjack\deck.h(19) : see declaration of 'Deck'
c:\visual studio 2005\projects\blackjack\blackjack\dealer.h(30) : error C2146: syntax error : missing ';' before identifier 'it'
c:\visual studio 2005\projects\blackjack\blackjack\dealer.h(30) : error C2065: 'it' : undeclared identifier
c:\visual studio 2005\projects\blackjack\blackjack\blackjack.cpp(11) : warning C4244: 'argument' : conversion from 'time_t' to 'unsigned int', possible loss of data
Build log was saved at "file://c:\Visual Studio 2005\Projects\Blackjack\Blackjack\Debug\BuildLog.htm"
Blackjack - 4 error(s), 1 warning(s)
========== Build: 0 succeeded, 1 failed, 0 up-to-date, 0 skipped ==========

---

Anyone have any ideas why this is happening?

deck.cards is actually a vector<Card>

Use vector<Card>::iterator.

Also, what are you trying to do with it = deck.cards[howmanycards] inside the for loop control?

I'm trying to make the loop stop when it reaches cards[howmanycards]

Syntax mistake I assume? It is giving me errors :d

Code:

---

c:\visual studio 2005\projects\blackjack\blackjack\dealer.h(31) : error C2679: binary '=' : no operator found which takes a right-hand operand of type 'Card' (or there is no acceptable conversion)

---

Okay, so I figured out the loop part, but now I'm having other trouble, when i'm pushing back cards into the hand.

Code:

---

        void Deal(int howmanycards, vector<Card> hand, vector<int>)
        {
                vector<Card>::iterator it = deck.cards.begin();
                for (it; it != deck.cards.begin() + howmanycards; it++)
                {
                        hand.push_back(deck.cards[it]);
                        deck.cards.erase(it);
                }
        }

---

And the error it produces

Code:

---

c:\visual studio 2005\projects\blackjack\blackjack\dealer.h(33) : error C2679: binary '[' : no operator found which takes a right-hand operand of type 'std::_Vector_iterator<_Ty,_Alloc>' (or there is no acceptable conversion)

---

Quote:

---

Originally Posted by Shamino

Code:

---

c:\visual studio 2005\projects\blackjack\blackjack\dealer.h(31) : error C2679: binary '=' : no operator found which takes a right-hand operand of type 'Card' (or there is no acceptable conversion)

---

---

Should be:

Code:

---

for (; *it != deck.cards.end(); ++it)

---

I would loop with a regular index.

If you want to use iterators, then you have to do iterator math to get the "end" iterator for your loop (edit: apparently you found that already):

Code:

---

vector<Card>::iterator end = deck.cards.begin() + howmanycards;

---

Unfortunately, on closer inspection of what you're doing that won't really work because you are erasing the items as you iterate. That is a bad idea.

One solution is to not iterate over the vector, but instead loop through howmanycards times and pop_back() from the end. This is much more efficient than erasing. The difference is that it means that you are dealing from the back of the deck (which is fine if you will be erasing from it).

Another solution is to not erase from the vector<Card> at all, but rather keep track of the current position in the deck. This is what I did for a toy Deck class I made. Your Deck class would need a variable to remember the current position in the vector so that it could find the next card. Then you could just loop through howmanycards times and increment that position as you go.

BTW, hand.push_back(deck.cards[it]); should be hand.push_back(*it);. Once you have an iterator you don't need to refer to the container to get it's value, just dereference it.

When I pop back, does that return a type the same as the vector? or does it just erase?

I'm trying to figure out how I would go about pop_back from the deck and then pushing that into the hand.

Use back() to get the value first (push that directly on to the hands vector). Then call pop_back() to erase it.

sweet!

Code:

---

        void Deal(int howmanycards, vector<Card> hand, vector<int>)
        {
                for (int loop = 0; loop = howmanycards; loop++)
                {
                        hand.push_back(deck.cards.back());
                        deck.cards.pop_back();
                }
        }

---

That works just fine it seems :).

You have one bug (I'm surprised it works). Look again at the stopping condition in the for control.

Also, consider checking to make sure howmanycards is less than or equal to the size of the deck before you start looping.

lol when i say works i mean it compiles..

I'm pretty lazy when it comes to testing things bahaha :D

I'll switch that up to a != bahaha

Quote:

---

Another solution is to not erase from the vector<Card> at all, but rather keep track of the current position in the deck. This is what I did for a toy Deck class I made. Your Deck class would need a variable to remember the current position in the vector so that it could find the next card. Then you could just loop through howmanycards times and increment that position as you go.

---

I would also suggest that idea. The thing is that your code seems to be trying to simulate very closely what's happening in the real world. However, if you encapsulate the knowledge which card is the "topmost" and dealt next into the Deck class, the underlying implementation can be simpler.

Here's a kind of a setup I have used.

Code:

---

#include <iostream>
#include <string>
#include <vector>
#include <cassert>

class Card
{
    int value;
    int suit;
    public:
        Card(int v, int s): value(v), suit(s) {}
        void display() const
        {
            std::cout << suit << " " << value << '\n';
        }
};

class Deck
{
    std::vector<Card> cards;
    std::size_t current;
    public:
        Deck();
        void shuffle();
        Card draw();
        std::size_t cards_left() const
        {
            return current;
        }
};

class Hand
{
    std::string name;
    std::vector<Card> cards;
    public:
        Hand(const std::string& name): name(name) {}
        void get_card(const Card& c)
        {
            cards.push_back(c);
        }
        void display() const;
};

void deal_cards(Deck& deck, Hand& hand, unsigned n)
{
    for (unsigned i = 0; i != n; ++i) {
        if (!deck.cards_left()) {
            std::cout << "Can't draw more cards\n";
            break;
        }
        hand.get_card(deck.draw());
    }
}

int main()
{
    Deck deck;
    Hand hand("John's hand");
    unsigned n;
    std::cout << "How many cards to take? ";
    std::cin >> n;
    deal_cards(deck, hand, n);
    hand.display();
}

Deck::Deck()
{
    cards.reserve(52);
    for (int i = 0; i != 13; ++i) {
        for (int j = 0; j != 4; ++j) {
            cards.push_back(Card(i, j));
        }
    }
    shuffle();
}

void Deck::shuffle()
{
    std::random_shuffle(cards.begin(), cards.end());
    current = cards.size();
}

Card Deck::draw()
{
    //in blackjack the cards might be reshuffled at that
    //point: after all more than one deckful of cards
    //would be in the game
    assert(current != 0 && "Deck::draw - no cards left");
    return cards[--current];
}

void Hand::display() const
{
    std::cout << name << '\n';
    for (std::vector<Card>::const_iterator it = cards.begin();
        it != cards.end(); ++it)
    {
        it->display();
    }
}

---

Well actually anon your method isn't really different from the real world at all.

The deck may not actually "tell" the dealer which is the top card, but the dealer certainly can "read" the deck to know what the top card is.

So a return value makes sense as long as it is the dealer class that is actually calling the function in my opinion.

But then I also think that removing items from the deck as I draw them makes more logical sense as well. In my new method I got rid of iterators and the erase() function as it was a bit too messy in terms of code, I'd have to take alot of extra steps in terms of error protection.

Two lines of code in a loop is able to manage drawing cards.

I guess I just find it better to actually emulate the real world than to make mathematical hacks to represent what is going on (not that your method will not work, it's just a question of style and design. Essentially once you draw those cards from the deck they don't exist, why fake it? Thats just my programmer theory.

Quote:

---

Essentially once you draw those cards from the deck they don't exist, why fake it?

---

Well, one idea is that used cards don't exist anymore as far as the rest of the code is concerned. However, you can cheaply get the whole deck back by reshuffling it (no need to construct again).

Another idea is that it relieves the calling code from managing the internals of a deck. The user - be it a Dealer object or Game object - gets to do some simple operations such as drawing a card from the deck, reshuffling or checking how many cards are left.