std::list::empty not working as expected

[joatmon]

The following snippet is intended to loop through a std::list until it finds the list to be empty. However, in my case, it is only entering the loop once and processing the first element. There are 7 elements in the list before this functions and 6 after it finishes.

    Card Player::clear_hand()
    {
        Card top_card;
        while (!my_hand.empty())
        {
            top_card = my_hand.front();
            my_hand.pop_front();
            return top_card;
        }
    }

my_hand is a std::list.

Can anybody see why this is only executing once (instead of 7 times)?

Thanks.

[Disch]

'return' exits the function (and therefore the loop)

If you just want to wipe the list and make it empty, it's easier to just use clear:

my_hand.clear();

Although, I'm not sure why you'd want clear_hand to return a card. What card are you returning?

[joatmon]

Thanks. I don't want to just clear the list, however. I need to send each item to another function for further processing. The function call looks like this:

discard_pile.collect_cards(p1.clear_hand());

Can you suggest a way for me to return the first element of the list to the calling function, re-executing the loop until the list is empty?

Thanks.

[m4ster r0shi]

You can  simply  create a new list, swap it with your  hand  and return it.

Note that  clear_hand  should have  list<Card>  as its  return  type and
collect_cards should accept one argument of type const list<Card> &.

Then, in collect_cards, you can simply use the splice member function to add
the collected cards to the rest of the pile (which should also be a list<Card>).

Useful links:

http://cplusplus.com/reference/stl/list/swap/
http://cplusplus.com/reference/stl/list/splice/

EDIT:

A correction.

If you want to use splice in collect_cards you should accept a non - const
argument. And since you can't bind a temporary (the result of clear_hand)
to  a  non - const  reference,  collect_cards  should accept  a  list<Card>.

[joatmon]

Thanks. I actually figured a lot of that out on my own (slowly, but surely...)

Rather than using splice, I'm using merge. Seems slightly simpler, particularly because I want to keep the original order for testing. However, it's not quite working, despite everything looking like it's set up correctly.

my function call is the same discard_pile.collect_cards(p1.clear_hand());

my revised function the Deck class looks like this:

    void Deck::collect_cards(list<Card> this_hand)
    {
//        list<Card>::iterator it;
//        for (it=this_hand.begin(); it!=this_hand.end(); it++) cout << " \n" << *it;
        my_deck.merge(this_hand);
    }

When I uncomment the iterator stuff, it all outputs correctly, so I know that I have the player's hand coming back into collect_cards correctly. (I also know that I cleared the original hand out, too). But, this still won't compile. It fails on the merge(this_hand) call. (note: my_deck is a std::list<Card> as is this_hand).

Can you see why? I'm just not seeing it.

Thanks as always.

[m4ster r0shi]

joatmon wrote:

I'm using merge [...] because I want to keep the original order for testing.

Don't worry about that. Using splice will just append your hand to your deck.
You should use merge only if you want your cards to be sorted in some way.

Try replacing the merge call with this -> my_deck.splice(my_deck.end(), this_hand);