[RESOLVED] polymorfic sort, overloading operator >

[juanpast]

Hi gurus.

I need to create a class witch it's operator > are polymorfical.

This is the code of .h file:

#define CRITERIO_ORDENACION(Fn,a,b,c,d,e) UCHAR (* Fn[5])(const CElementoInstruccion &, const CElementoInstruccion &) = {a,b,c,d,e}

#define STEP CheckStep
#define INSTRUCCION CheckInstruccion 
#define ELEMENTO CheckElemento
#define DISTANCIA CheckDistancia
#define TRAMO CheckTramo

class CElementoInstruccion {	
	friend bool operator == (const CElementoInstruccion &ei1, const CElementoInstruccion &ei2);
	friend bool operator != (const CElementoInstruccion &ei1, const CElementoInstruccion &ei2);
	friend bool operator > (const CElementoInstruccion &ei1, const CElementoInstruccion &ei2);
	friend bool operator < (const CElementoInstruccion &ei1, const CElementoInstruccion &ei2);
	friend bool operator >= (const CElementoInstruccion &ei1, const CElementoInstruccion &ei2);
	friend bool operator <= (const CElementoInstruccion &ei1, const CElementoInstruccion &ei2);
private: 
	void Copy(const CElementoInstruccion &ei);
	void InicializaOrden(void);
	UCHAR (* Criterio_Ordenacion[5])(const CElementoInstruccion &, const CElementoInstruccion &);
public:
	CElementoInstruccion();
	~CElementoInstruccion();
	UINT nStep, nInstruccion, nTramo, nDistancia, nElemento;

	void SetCriterioOrdenacion(UCHAR (* Criterio_Ordenacion[5])(const CElementoInstruccion &, const CElementoInstruccion &));

	CElementoInstruccion & operator =(const CElementoInstruccion &ei);
	CElementoInstruccion(const CElementoInstruccion &ei);

	bool operator == (const CElementoInstruccion &ei);
	bool operator != (const CElementoInstruccion &ei);
	bool operator > (const CElementoInstruccion &ei);
	bool operator < (const CElementoInstruccion &ei);
	bool operator >= (const CElementoInstruccion &ei);
	bool operator <= (const CElementoInstruccion &ei);	
};


UCHAR CheckStep(const CElementoInstruccion &ei1, const CElementoInstruccion &ei2);
UCHAR CheckInstruccion(const CElementoInstruccion &ei1, const CElementoInstruccion &ei2);
UCHAR CheckElemento(const CElementoInstruccion &ei1, const CElementoInstruccion &ei2);
UCHAR CheckDistancia(const CElementoInstruccion &ei1, const CElementoInstruccion &ei2);
UCHAR CheckTramo(const CElementoInstruccion &ei1, const CElementoInstruccion &ei2);

And this is the code for .cpp file:

using namespace std;

void CElementoInstruccion::InicializaOrden(void) {		
	Criterio_Ordenacion[0] = STEP;
	Criterio_Ordenacion[1] = INSTRUCCION;
	Criterio_Ordenacion[2] = ELEMENTO;
	Criterio_Ordenacion[3] = DISTANCIA;
	Criterio_Ordenacion[4] = TRAMO;
}

CElementoInstruccion::CElementoInstruccion() {
	InicializaOrden();
	nStep = 0;
	nInstruccion = 0;
	nTramo = 0;
	nDistancia = 0;
	nElemento = 0;
}

CElementoInstruccion::~CElementoInstruccion() {
}

void CElementoInstruccion::Copy(const CElementoInstruccion &ei) {
	for(int i=0;i<5;i++) Criterio_Ordenacion[i] = ei.Criterio_Ordenacion[i];
	nStep = ei.nStep;
	nInstruccion = ei.nInstruccion;
	nTramo = ei.nTramo;
	nDistancia = ei.nDistancia;
	nElemento = ei.nElemento;
}

CElementoInstruccion & CElementoInstruccion::operator =(const CElementoInstruccion &ei) {
	Copy(ei);
	return *this;
}

CElementoInstruccion::CElementoInstruccion(const CElementoInstruccion &ei) {
	Copy(ei);
}

bool CElementoInstruccion::operator == (const CElementoInstruccion &ei) {
	return nStep == ei.nStep && nInstruccion == ei.nInstruccion && nTramo == ei.nTramo && nDistancia == ei.nDistancia && nElemento == ei.nElemento;
}

bool CElementoInstruccion::operator != (const CElementoInstruccion &ei) {
	return !operator==(ei);
}

bool CElementoInstruccion::operator > (const CElementoInstruccion &ei) {	
	if(Criterio_Ordenacion[0](*this, ei) == 0) return true;
	if(Criterio_Ordenacion[0](*this, ei) == 1) return false;

	if(Criterio_Ordenacion[1](*this, ei) == 0) return true;
	if(Criterio_Ordenacion[1](*this, ei) == 1) return false;

	if(Criterio_Ordenacion[2](*this, ei) == 0) return true;
	if(Criterio_Ordenacion[2](*this, ei) == 1) return false;

	if(Criterio_Ordenacion[3](*this, ei) == 0) return true;
	if(Criterio_Ordenacion[3](*this, ei) == 1) return false;

	if(Criterio_Ordenacion[4](*this, ei) == 0) return true;
	if(Criterio_Ordenacion[4](*this, ei) == 1) return false;

	return false;
}

UCHAR CheckStep(const CElementoInstruccion &ei1, const CElementoInstruccion &ei2) {
	if(ei1.nStep > ei2.nStep) return 0;
	if(ei1.nStep < ei2.nStep) return 1;
	return 2;
}

UCHAR CheckInstruccion(const CElementoInstruccion &ei1, const CElementoInstruccion &ei2) {
	if(ei1.nInstruccion > ei2.nInstruccion) return 0;
	if(ei1.nInstruccion < ei2.nInstruccion) return 1;
	return 2;
}

UCHAR CheckElemento(const CElementoInstruccion &ei1, const CElementoInstruccion &ei2) {
	if(ei1.nElemento > ei2.nElemento) return 0;
	if(ei1.nElemento < ei2.nElemento) return 1;
	return 2;
}

UCHAR CheckDistancia(const CElementoInstruccion &ei1, const CElementoInstruccion &ei2) {
	if(ei1.nDistancia > ei2.nDistancia) return 0;
	if(ei1.nDistancia < ei2.nDistancia) return 1;
	return 2;
}

UCHAR CheckTramo(const CElementoInstruccion &ei1, const CElementoInstruccion &ei2) {
	if(ei1.nTramo > ei2.nTramo) return true;
	if(ei1.nTramo < ei2.nTramo) return false;
	return 2;
}


bool CElementoInstruccion::operator < (const CElementoInstruccion &ei) {
	if(operator==(ei)) return false;
	return !operator > (ei);
}

bool CElementoInstruccion::operator >= (const CElementoInstruccion &ei) {
	if(operator > (ei)) return true;
	if(operator == (ei)) return true;
	return false;
}

bool CElementoInstruccion::operator <= (const CElementoInstruccion &ei) {
	if(operator < (ei)) return true;
	if(operator == (ei)) return true;
	return false;
}

void CElementoInstruccion::SetCriterioOrdenacion(UCHAR (* FX_Orden[5])(const CElementoInstruccion &, const CElementoInstruccion &)) {
	Criterio_Ordenacion[0] = FX_Orden[0];
	Criterio_Ordenacion[1] = FX_Orden[1];
	Criterio_Ordenacion[2] = FX_Orden[2];
	Criterio_Ordenacion[3] = FX_Orden[3];
	Criterio_Ordenacion[4] = FX_Orden[4];
}


bool operator == (const CElementoInstruccion &ei1, const CElementoInstruccion &ei2) {	
	return ei1.nStep == ei2.nStep && ei1.nInstruccion == ei2.nInstruccion && ei1.nTramo == ei2.nTramo && ei1.nDistancia == ei2.nDistancia && ei1.nElemento == ei2.nElemento;
}

bool operator != (const CElementoInstruccion &ei1, const CElementoInstruccion &ei2) {
	return !operator ==(ei1, ei2);
}

bool operator > (const CElementoInstruccion &ei1, const CElementoInstruccion &ei2) {

	if(ei1.Criterio_Ordenacion[0](ei1, ei2) == 0) return true;
	if(ei1.Criterio_Ordenacion[0](ei1, ei2) == 1) return false;

	if(ei1.Criterio_Ordenacion[1](ei1, ei2) == 0) return true;
	if(ei1.Criterio_Ordenacion[1](ei1, ei2) == 1) return false;

	if(ei1.Criterio_Ordenacion[2](ei1, ei2) == 0) return true;
	if(ei1.Criterio_Ordenacion[2](ei1, ei2) == 1) return false;

	if(ei1.Criterio_Ordenacion[3](ei1, ei2) == 0) return true;
	if(ei1.Criterio_Ordenacion[3](ei1, ei2) == 1) return false;

	if(ei1.Criterio_Ordenacion[4](ei1, ei2) == 0) return true;
	if(ei1.Criterio_Ordenacion[4](ei1, ei2) == 1) return false;
	
	return false;
}

bool operator < (const CElementoInstruccion &ei1, const CElementoInstruccion &ei2) {
	if(operator==(ei1, ei2)) return false;
	return !operator > (ei1, ei2);
}

bool operator >= (const CElementoInstruccion &ei1, const CElementoInstruccion &ei2) {
	if(operator > (ei1, ei2)) return true;
	if(operator == (ei1, ei2)) return true;
	return false;
}

bool operator <= (const CElementoInstruccion &ei1, const CElementoInstruccion &ei2) {
	if(operator < (ei1, ei2)) return true;
	if(operator == (ei1, ei2)) return true;
	return false;
}

I want to make this as static member (.h file, remarks in red and bold):

UCHAR (* Criterio_Ordenacion[5])(const CElementoInstruccion &, const CElementoInstruccion &);

If this is a static member y don't need to change the order for each object, only in the class. But I don't know how to do this...


Best regards and thank you in advance.

[monarch_dodra]

Inside your cpp:

UCHAR (* CElementoInstruccion::Criterio_Ordenacion[5])(const CElementoInstruccion &, const CElementoInstruccion &) = {
  STEP,
  INSTRUCCION,
  ELEMENTO,
  DISTANCIA,
  TRAMO
};

or, if you want to null-initialize it.

UCHAR (* CElementoInstruccion::Criterio_Ordenacion[5])(const CElementoInstruccion &, const CElementoInstruccion &) =
{0};

Not entirely sure what you are doing is the best approach though... Too tired to think right now...

[D_Drmmr]

This is the code of .h file:

You would make everybody's life a lot easier if you got rid of all the redundant code.
AFAICT, you don't need the destructor, copy c'tor and assignment operator as the default implementation will do just fine.
Also, you have defined two overloads for each operator, one outside the class and one as class member (with the left-hand-side non-const). You can get rid of one of the two.
The implementation of the operators is also confusing. If you want to define operator < in terms of operator >, then just reverse the operators. There is no need for an extra check. Same for the implementation of operator >. You are calling each comparison function twice and you copy-pasted code instead of just writing a loop.

I want to make this as static member (.h file, remarks in red and bold):

UCHAR (* Criterio_Ordenacion[5])(const CElementoInstruccion &, const CElementoInstruccion &);

If this is a static member y don't need to change the order for each object, only in the class. But I don't know how to do this...

Just prefix the declaration in the class with the static keyword and then add a definition in the .cpp file. I assume you'll want to make the function that sets this member static as well.

From an interface point-of-view, it might be easier to let the user set an array of enum values instead of an array of function pointers. Internally, you can still map the enum values to function pointers, but the interface of the class would become much neater.

[Paul McKenzie]

Hi gurus.

I need to create a class witch it's operator > are polymorfical.

From a C++ point of view, you are overloading the wrong relational operator, and too many operators. As D_Drmmr points out, you have too much code, and what I'm suggesting to you makes the code even shorter.

There are two operators you should overload fully: operator == (which you did), and operator <, which you did not do. Operator < should be where the coding should be, and operator > should be written in terms of operator <. However, the other operators you overloaded need not be written at all, including operator >.

The reasons why operator < and operator == are the major players in C++ are two-fold:

1) Most, if not all C++ algorithms uses operator < and operator == for comparisons, sorts, etc. Rarely if ever is operator > and operator == used internally by any of the algorithm functions.

2) Standard c++ has a std::rel_ops namespace. This allows you to define all of the other relational operators without you having to write any extra code, given that you've defined operator < and operator ==.

http://www.cplusplus.com/reference/std/utility/rel_ops/

Example:

#include <utility>

class CElementoInstruccion {	
// only these two operators need to be defined
	friend bool operator == (const CElementoInstruccion &ei1, const CElementoInstruccion &ei2);
	friend bool operator < (const CElementoInstruccion &ei1, const CElementoInstruccion &ei2);
};

//...
using namespace rel_ops;

int main()
{
   // your code using CElementoInstruccion goes here
}

Since all properly coded operator >, >=, !=, and <= follow the exact same pattern when operator < and operator == are defined fully, the rel_ops templates encapsulates this behaviour.

Regards,

Paul McKenzie

[nuzzle]

If this is a static member y don't need to change the order for each object, only in the class. But I don't know how to do this...

Well, what if this class is used from several threads and each thread wants a different sort order at the same time? Disaster!

A class should have a natural order that never changes. If some other order is wanted for sorting it should be passed-in to the sorting algorithm (which then overrides the natural order in favour of the passed-in order).

[juanpast]

Thanks.

Your notes are very appreciated and useful.

2) Standard c++ has a std::rel_ops namespace. This allows you to define all of the other relational operators without you having to write any extra code, given that you've defined operator < and operator =.

Good!

Well, what if this class is used from several threads and each thread wants a different sort order at the same time? Disaster!

Upssss, good note!

Very thanks!!

[monarch_dodra]

2) Standard c++ has a std::rel_ops namespace. This allows you to define all of the other relational operators without you having to write any extra code, given that you've defined operator < and operator =.

Not a fan of rel_ops myself as it basically sets a program-wide behavior. I think boost::operators is a better alternative. But to each their own I guess.

----
To the OP:

1)Why use a 5-array? If you use a vector your code will be:
1.1)Easier to maintain
1.2)More expandable: You can choose any number of comparison schemes, from less than 5 to more than 5.

2)As stated, you are introducing a global state, which is never good. You might be better off using a stateful comparison function object:

#include <vector>

//Typedef: Always use typedefs
typedef UCHAR (* Criterio_Ordenacion_t )(const CElementoInstruccion &, const CElementoInstruccion &);

class CElementoInstruccionComparatorLess
{
public:
  CElementoInstruccionComparatorLess()
  { }

  void push_comparator(Criterio_Ordenacion_t i)
  {m_comparators.push_back(i);}

  bool operator()(const CElementoInstruccion& i_lhs, const CElementoInstruccion& i_rhs)
  {
    const size_t size = m_comparators.size();
    for(size_t i = 0 ; i < size ; ++i)
    {
      if(m_comparators[i](i_lhs, i_rhs) == 0) {return true;}
      if(m_comparators[i](i_lhs, i_rhs) == 1) {return false;}
    }
    //If we reach here, they are equivalent
    return true;
  }

private:
  std::vector<Criterio_Ordenacion_t> m_comparators;
};

int main()
{
  CElementoInstruccion a, b;

  CElementoInstruccionComparatorLess CompareBySTEP;
  CompareBySTEP.push_comparator(STEP);


  CElementoInstruccionComparatorLess CompareByINSTRUCCIONandELEMENTO;
  CompareBySTEP.push_comparator(INSTRUCCION);
  CompareBySTEP.push_comparator(ELEMENTO);

  CompareBySTEP(a, b);
  CompareByINSTRUCCIONandELEMENTO(a, b);
}

The advantage here is that you can explicitly create your comparator. You can have several co-exist, and you can pass them to functions (eg algorithm).