std::iterator error!

This is a discussion on std::iterator error! within the C++ Programming forums, part of the General Programming Boards category; I'm trying to make an iterator for the cards vector but it doesn't seem to be working correctly, here's the ...

  1. #1
    Absent Minded Programmer
    Join Date
    May 2005
    Posts
    964

    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>
    Sometimes I forget what I am doing when I enter a room, actually, quite often.

  2. #2
    Registered User
    Join Date
    Jan 2005
    Posts
    7,328
    Use vector<Card>::iterator.

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

  3. #3
    Absent Minded Programmer
    Join Date
    May 2005
    Posts
    964
    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)
    Sometimes I forget what I am doing when I enter a room, actually, quite often.

  4. #4
    Absent Minded Programmer
    Join Date
    May 2005
    Posts
    964
    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)
    Sometimes I forget what I am doing when I enter a room, actually, quite often.

  5. #5
    Captain Crash brewbuck's Avatar
    Join Date
    Mar 2007
    Location
    Portland, OR
    Posts
    7,243
    Quote Originally Posted by Shamino View Post
    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)

  6. #6
    Registered User
    Join Date
    Jan 2005
    Posts
    7,328
    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.

  7. #7
    Absent Minded Programmer
    Join Date
    May 2005
    Posts
    964
    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.
    Sometimes I forget what I am doing when I enter a room, actually, quite often.

  8. #8
    Registered User
    Join Date
    Jan 2005
    Posts
    7,328
    Use back() to get the value first (push that directly on to the hands vector). Then call pop_back() to erase it.

  9. #9
    Absent Minded Programmer
    Join Date
    May 2005
    Posts
    964
    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 .
    Sometimes I forget what I am doing when I enter a room, actually, quite often.

  10. #10
    Registered User
    Join Date
    Jan 2005
    Posts
    7,328
    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.

  11. #11
    Absent Minded Programmer
    Join Date
    May 2005
    Posts
    964
    lol when i say works i mean it compiles..

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

    I'll switch that up to a != bahaha
    Sometimes I forget what I am doing when I enter a room, actually, quite often.

  12. #12
    The larch
    Join Date
    May 2006
    Posts
    3,573
    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();
        }
    }
    Last edited by anon; 11-13-2007 at 04:09 PM.
    I might be wrong.

    Thank you, anon. You sure know how to recognize different types of trees from quite a long way away.
    Quoted more than 1000 times (I hope).

  13. #13
    Absent Minded Programmer
    Join Date
    May 2005
    Posts
    964
    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.
    Last edited by Shamino; 11-13-2007 at 04:52 PM.
    Sometimes I forget what I am doing when I enter a room, actually, quite often.

  14. #14
    The larch
    Join Date
    May 2006
    Posts
    3,573
    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.
    I might be wrong.

    Thank you, anon. You sure know how to recognize different types of trees from quite a long way away.
    Quoted more than 1000 times (I hope).

Popular pages Recent additions subscribe to a feed

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21