C ++ Custom comparison function for list :: sort

Question

Hi, I am having trouble compiling a simple piece of code. I am creating a class that implements a deck of cards, and I want to create a shuffle method using the list :: short method.

Relevant Code:

deck.h

#ifndef _DECK_H #define _DECK_H #include <list> #include <ostream> #include "Card.h" #include "RandomGenerator.h" using namespace std; class Deck { private: static const int CARD_NUMBER = Card::CARDS_PER_SUIT*Card::SUIT_NUMBER; list<Card *> *cards; RandomGenerator rg; public: Deck(); ~Deck(); void shuffle(); private: bool const compareRandom(const Card *a, const Card *b); }; #endif /* _DECK_H */ 

deck.cc:

 #include "Deck.h" /** * Fills the deck with a set of 52 cards */ Deck::Deck() { cards = new list<Card *>(); for(int i = 0; i < CARD_NUMBER; i++) cards->push_back( new Card( Card::Suit(int(i/Card::CARDS_PER_SUIT)), i%Card::CARDS_PER_SUIT) ); } Deck::~Deck() { gather(); for(list<Card *>::iterator c = cards->begin(); c != cards->end(); c++) delete *c; delete cards; } bool const Deck::compareRandom(const Card *a, const Card *b) { return rg.randomBool(); } void Deck::shuffle() { cards->sort(compareRandom); } 

The compiler displays the following message (ignores line numbers):

 Deck.cc: In member function 'void Deck::shuffle()': Deck.cc:66: error: no matching function for call to 'std::list<Card*, std::allocator<Card*> >::sort(<unresolved overloaded function type>)' /usr/include/c++/4.3/bits/list.tcc:303: note: candidates are: void std::list<_Tp, _Alloc>::sort() [with _Tp = Card*, _Alloc = std::allocator<Card*>] /usr/include/c++/4.3/bits/list.tcc:380: note: void std::list<_Tp, _Alloc>::sort(_StrictWeakOrdering) [with _StrictWeakOrdering = const bool (Deck::*)(const Card*, const Card*), _Tp = Card*, _Alloc = std::allocator<Card*>] 

The problem should be on the compareRandom link, which I am using incorrectly, I can not find the answer to this problem.

Thanks in advance.

Answer 1

Can i say something :)

First, do not store the pointer to the Card , just store the cards directly in the container. If you insist on keeping pointers to them for any reason, use shared_ptr<Card> from Boost . Secondly, you can use std::random_shuffle and pass you random-number-generator instead of implementing your shuffle function.

---

Can I say something else again :)

This is what I mean if you do not need to use list for any reason, although I do not see this reason.

 #include <iostream> #include <vector> #include <deque> #include <algorithm> class Card { // ... }; int main() { typedef std::vector<Card> Deck; Deck deck; // ... fill deck with cards. // There is an optional third parameter, // if you need to pass YOUR random-number-generator! // If you do, I recommend Boost implementation. std::random_shuffle(deck.begin(), deck.end()); } 

I like to handle containers directly in C++ , although you may not like it. Also, if you see that std::vector has performance issues in your case, you can simply replace typedef with std::deque :

 typedef std::deque<Card> Deck; 

Answer 2

compareRandom is a member function, it has type bool (Deck::*)(const Card*, const Card*) , which means you cannot call it like f(a,b) , what sorting will be. You can make compareRandom a static or standalone function, or use a functor to adapt it to a specific Deck instance.

Answer 3

BTW - you cannot mix with sort :) Sorting makes some assumptions about the comparison function.

Answer 4

Besides what others said: you can use std::shuffle std::random_shuffle (something I found out today, cheers!), I would add that you cannot use a random function as a sort criterion.

sort takes a strict weak order as a comparator, which means that if a < b (or compareRandom(a,b) returns false, then b < a ( compareRandom(b,a) returns true), and b == a should return false, which you can never be guaranteed by a random function. In this case, the sort behavior is undefined. I don’t know if it even ends ...

Answer 5

The cause of the error is Logan Capaldo's answer . Now you can replace compareRandom with a functor as follows:

 ... private: struct compareRandom { // it shouldn't give a random compare result. // sort will not work (in Visual C++ 2008 it gives runtime assert) bool operator()(const Card *a, const Card *b) { return rg.randomBool(); } }; ... 

Then use it

 void Deck::shuffle() { cards->sort( compareRandom() ); } 

Answer 6

I urge you to use std::random_shuffle . It will not work with list , but it will work with deque or vector , so if you do not need list properties, I suggest you use a different container. If you must use a list, try the following:

 void Deck::shuffle() { vector<Card*> temp(cards->begin(), cards->end()); random_shuffle(temp.begin(), temp.end()); cards->assign(temp.begin(), temp.end()); }