Re: sorting stl list with predicate
On Jan 28, 3:38 pm, ManicQin <Manic...@gmail.com> wrote:
I have a list of COperation* and I add Operations, each
Operation has a scale and before I execute the list i want to
sort them
I have the next snip:
class COperation
{
public:
virtual int GetScale() const { return -1; }
};
typedef COperation* PCOperation;
typedef std::list<PCOperation> tOpStack;
struct greaterScale : public std::greater<PCOperation>
{
bool operator()(const PCOperation& x,const PCOperation& y)
{
if (x->GetScale() != -1 && y->GetScale() != -1 && y->=
GetScale() > x->GetScale())
{
return true;
}
return false;
}
};
later I try to call:
tOpStack m_OpStack;
m_OpStack.sort(greaterScale());
That's not a legal predicate for sort. Sort (and all other
ordering functions in the standard library) require that the
expression pred(a, b) && pred(b, a) define an equivalence
relationship. Equivalence relationships must be transitive,
i.e. a===b and b===c iff a===c. Your predicate doesn't h=
ave
this characteristic---if b.GetScale() returns -1, b will be
equivalent to a and c, even if a and c aren't equivalent.
when I enter the sort I see that instead of using my
greaterScale pred he is using the normal greater pred...
That would surprise me. How do you conclude this?
I'm guessing that my greaterScale isn't using the correct
signature and the compiler cant deduce the correct operator()
but why?
The greaterScale::operator() should be const, so you formally
have undefined behavior. But in practice, if it compiles, it
will work.
what am I doing wrong?
Not fulfilling the requirements on the ordering predicate.
Resulting in undefined behavior.
A few other minor comments (not related to your actual problem,
but important for writing good code):
-- The C prefix for classes is used by MFC. It's best to avoid
it in your own code. (Now that we have namespaces, there's
not really a need of a prefix anyway.)
-- The typedef for the pointer is more obfuscation than
anything else. If you're using pointers, the reader should
be able to easily see this. (The typedef might be justified
in order to easily migrate to boost::shared_ptr, or some
other intelligent pointer. In that case, however, the name
should indicate that it is a pointer, e.g. OperationPointer.
My own convention in such cases is to use a typedef in the
class, e.g. in class Operation, something like:
typedef Operation* Pointer ;
or
typedef boost::shared_ptr< Operation > Pointer ;
This results in Operation::Pointer, which seems reasonably
readable.)
-- And using a single if to return true or false is confusing
as well, at least to anyone who knows what a bool is. Just
return the expression.
--
James Kanze (GABI Software) email:james.kanze@gmail.com
Conseils en informatique orient=E9e objet/
Beratung in objektorientierter Datenverarbeitung
9 place S=E9mard, 78210 St.-Cyr-l'=C9cole, France, +33 (0)1 30 23 00 34