Kritik und Optimierungen (Klassen mit Operatoroverloading)

BorisDieKlinge

Code:

class CRoute{

		CTypedPtrArray<CPtrArray,CGroup*> m_paNode;

	public:
		//Konstruktor
		CRoute(CGroup *p){
			m_paNode.Add(p);
		}
		CRoute(){}
		//Destruktor
		~CRoute(){
			m_paNode.RemoveAll();
		}
		inline int GetSize() { 
			return m_paNode.GetSize(); 
		}
		inline CGroup* operator[] (size_t iIndex){
			return m_paNode.ElementAt(iIndex);
		}
		/*inline void Add(CGroup *p){
			m_paNode.Add(p);
		}*/
		//Route an Route anhängen!
		void operator+(CRoute *p){
			for(int k=0; k< p->GetSize(); k++)
				this->m_paNode.Add((*p)[k]);
		}
		//Gruppe an Route
		void operator+(CGroup *p){
			this->m_paNode.Add(p);
		}
		//Route zuweisen (Alte Daten werden gelöscht)
		CRoute& operator=(CRoute *p){
			this->m_paNode.RemoveAll();
			if(p==NULL)
				return *this;
			for(int i=0; i< p->GetSize(); i++)
				this->m_paNode.Add((*p)[i]);
			return *this;
		}
		//
		CRoute& operator+=(CRoute *p){
			if(p==NULL)
				return *this;
			for(int k=0; k< p->GetSize(); k++)
				this->m_paNode.Add((*p)[k]);
			return *this;
		}
	};

würde mich freuen wenn ihr euere Meinung und Verbessungsvorschläge hinsichtlich des Code geben würded. Die klasse ist noch nich fertig, will nur wissen ob das mit den operatator overlading richtig ist.

Ich übergeben den operatoren nen Pointer der klasse und nicht eine Referenz auf das Objekt der Klasse. Ist das tragisch?

CStoll

Erstens: Auf jeden Fall sollten die Operatoren ihre Parameter per Referenz übernehmen

Zweitens: operator=() achtet nicht auf Selbstzuweisungen. Eine Anweisung ala "r1=r1;" (die kann sich auch hinter Referenzen/Zeigern verstecken) würde das Objekt löschen.

Drittens: Du solltest op+ lieber global und basierend auf op+= definieren (so wie ich es in deinem letzten Thread mit & und &= angedeutet habe).

PS: Und Methoden, die direkt in der Klassendefinition definiert werden, sind automatisch 'inline'

Hi, also ...

1.) Überlege ob du der Ein-Parameter-Konstruktor auch als Umwandlungsfunktion genutzt werden soll, wenn nicht dann explicit machen.

2.) Wenn die Klasse als Basisklasse benutzt wird und Polymorphie auch mit im Spiel ist dann Destruktor virtual machen.

3.) getSize const machen

4.) op[] ne Refernez zurückgeben lassen, damit man nicht derefernzieren muss.

5.) op+ außerhalb der Klasse und const& als Parameter und mit += implementieren und const Objekt zurückgeben lassen.

6.) op= Exceptionsicher machen, dh das Objekt erst ändern wenn alle Aufgaben erfolgreich erledigt wurden. Zuerst nen Temporäres Objekt erzeugen und dann swap und Parameter als const& wieder, oder direkt per Value und dann swap.

7.) Über einen CopyCtor nachdenken.

8.) op+= auch Exceptionsicher machen.

Das wars mal auf die schnelle Vorerst ...

Edit: To slow

BorisDieKlinge

d.h. wenn ich die parameter per refernez übergeben soll
muss ich später sowas wie bsp.

CRoute *a=...
Corute *b=...;

(*a)+=(*b);

umd den zeiger zu defrerenzeiren und dann als obejtk referen übergeben?

LordJaxom

Nein, wenn Du die Parameter per Referenz übergibst verhinderst Du lediglich dass eine Kopie erzeugt wird, bzw. stellst sicher dass bei modifizierenden Operatoren auch das Originalobjekt geändert wird und nicht bloß die Kopie. An den Aufrufen ändert sich garnichts.

BTW: Den Vergleich (*a) == (*b) musst Du hier sowieso machen, da a == b ein Zeigervergleich ist und der Vergleich von Builtin-Typen sich nicht überladen lässt.

BorisDieKlinge

also quasie den Pointer per referenz übergeben?

CStoll

BorisDieKlinge schrieb:

d.h. wenn ich die parameter per refernez übergeben soll
muss ich später sowas wie bsp.
CRoute *a=...
Corute *b=...;

(*a)+=(*b);
umd den zeiger zu defrerenzeiren und dann als obejtk referen übergeben?

Warum mußt du denn sowas machen? Im Allgemeinen ist es besser/sicherer, Objekte auf dem Stack abzulegen:

CRoute a,b;

a+=b;

PS: Ich weiß nicht, ob du mit Vererbung arbeitest. Wenn nicht, ist ein vector<CGroup> vermutlich einfacher (und auf jeden Fall portabler) als dein CTypedPtrArray.

BorisDieKlinge

ja arbeite noch mit MFC... und ich verwende vererbung;) deswegen nehm ich immer CTypedPtrArray:) So bin fertig mit areiten;) ich meld mich morgen wieder ..heheh

grüße und danke (besonders CStoll)

CStoll schrieb:

[...] Du solltest op+ lieber global und basierend auf op+= definieren [...]

Was bringt das?
Ist das schneller oder leichter zu programmieren???

Konrad Rudolph

AGS'ler schrieb:

CStoll schrieb:

[...] Du solltest op+ lieber global und basierend auf op+= definieren [...]

Was bringt das?
Ist das schneller oder leichter zu programmieren???

1. Es macht die "offizielle" Schnittstelle der Klasse kleiner, das ist immer positiv, schon aus Gründen der Wartbarkeit.

2. Es ahmt das Verhalten eingebauter Datentypen und derer Operationen besser nach.

3. Es erlaubt, implizite Konversion zu verwenden, um dem Programmierer Arbeit abzunehmen:

class foo;

foo a, b;
int c;

// Angenommen, folgendes soll gehen:
b = a + c;
// und
b = c + a;

// dann reicht es, den op+ in eine Richtung zu definieren und einen impliziten Konstruktor für foo zu definieren, der ein 'int' als Argument erhält.

BorisDieKlinge

Versteh ich nich: -> Du solltest op+ lieber global und basierend auf op+= definieren!

Folgednes Versteh ich auch nich:

1.Es macht die "offizielle" Schnittstelle der Klasse kleiner, das ist immer positiv, schon aus Gründen der Wartbarkeit.

2. Es ahmt das Verhalten eingebauter Datentypen und derer Operationen besser nach.

3. Es erlaubt, implizite Konversion zu verwenden, um dem Programmierer Arbeit abzunehmen:

class foo;

foo a, b;
int c;

// Angenommen, folgendes soll gehen:
b = a + c;
// und
b = c + a;

// dann reicht es, den op+ in eine Richtung zu definieren und einen impliziten Konstruktor für foo zu definieren, der ein 'int'

CStoll

Als Ergänzung zu Konrad:

Konrad Rudolph schrieb:

1. Es macht die "offizielle" Schnittstelle der Klasse kleiner, das ist immer positiv, schon aus Gründen der Wartbarkeit.

Wenn du etwas an der Semantik der Addition ändern willst, mußt du nur noch eine Funktion (op+=) anpassen - op+ übernimmt deren Verhalten dann automatisch.

2. Es ahmt das Verhalten eingebauter Datentypen und derer Operationen besser nach.

Für eingebaute Datentypen (int, double, Zeiger, etc) gilt laut Standard, daß "x+=y;" äquivalent zu "x=x+y;" ist. Indem du op+ über op+= definierst, erreichst du den selben Effekt für deine Klassen. ("do it as the int's" ist ein recht guter Vorsatz bei der Operator-Überladung ;))

(Der umgekehrte Weg, op+= durch op+ zu definieren, wäre zwar auch möglich, aber performance-technisch ungünstiger)

3. Es erlaubt, implizite Konversion zu verwenden, um dem Programmierer Arbeit abzunehmen:

Das hat jetzt nicht unbedingt etwas mit op+ vs. op+= zu tun, sondern mit der generellen Entscheidung "Operator global oder als Methode?" Wenn du den Operator als Methode deiner Klasse definierst, kann der Compiler keine Typumwandlungen für den linken Operanden vornehmen, d.h. ein Ausdruck wie T x=4711+y; ist nicht möglich. Bei einem globalen Operator ist der Compiler in der Lage, die 4711 (mit dem Ctor T(int)) in ein T umzuwandeln und dieses zu addieren.

BorisDieKlinge

aber was ist mit einem GLOBALEN Operator gemeint?

CStoll

Du kannst jeden Operator entweder als Memberfunktion T::operator+(const T& rhs); (der linke Operand ist *this) oder als globale Funktion^* operator+(const T& lhs,const T& rhs) anlegen.

^* mit einigen Ausnahmen - =, [], () und -> dürfen nur als Member angelegt werden (hab ich einen vergessen?)

Konrad Rudolph

BorisDieKlinge schrieb:

Folgednes Versteh ich auch nich:

Hm, wenn man versucht, etwas in Eile zu erklären, geht es halt immer schief. Sorry.

Aber CStoll hat ja im Prinzip schon die Rosinen rausgepickt.

CStoll schrieb:

Für eingebaute Datentypen (int, double, Zeiger, etc) gilt laut Standard, daß "x+=y;" äquivalent zu "x=x+y;" ist. Indem du op+ über op+= definierst, erreichst du den selben Effekt für deine Klassen. ("do it as the int's" ist ein recht guter Vorsatz bei der Operator-Überladung ;))

AFAIK gab es da auch noch ein paar Nuancen, was das Verhalten von lvalues und rvalues betraf. Weiß ich aber nicht mehr auswendig und campers bezüglichen Beitrag finde ich jetzt auf die Schnelle nicht.

BorisDieKlinge schrieb:

aber was ist mit einem GLOBALEN Operator gemeint?

Na, Operatoren, die außerhalb der Klasse definiert wurden.

class foo { };

foo operator +(foo const& a, foo const& b) {
    foo tmp = a;
    tmp += b;    // hier greifen wir auf foo::operator += zu
    return tmp;
}

BorisDieKlinge

ok , und wieso kann man:

foo operator +(foo const& a, foo const& b) {
    foo tmp = a;
    tmp += b;    // hier greifen wir auf foo::operator += zu
    return tmp;
}

nicht in einer Klasse deklarien? muss es global sein um es zu benutzen? Dachte ich hätte den operator schon mal in einer klasse verwendet!

CStoll

Klar kannst du den Operator auch in der Klasse definieren (in dem Fall entfällt der erste Parameter und du nutzt *this stattdessen) - der Grund, warum du es nicht tun solltest, sind die impliziten Konvertierungen:

class myNumber
{
public:
  myNumber(int);
...
  //a:
  myNumber operator+(const myNumber& r);
};

//b:
myNumber operator+(const myNumber& l, const myNumber& r);

...
myNumber a,b,c;

c=a+b;//klappt immer
c=a+1;//klappt auch (wandelt 1 in ein myNumber um und addiert)
c=1+b;//klappt nur mit Variante (b)
c=1+2;//klappt ;) (Bonusfrage: Kannst du mir sagen, welche Operatoren/Methoden hier verwendet werden?)

Kritisch ist in dem Beispiel die Anweisung c=1+b; - der Compiler darf keine impliziten Umwandlungen für this anwenden, deshalb lässt sich sowas mit dem Member-Operator nicht umsetzen.

BorisDieKlinge

"1+2" : Denk mal globater + operator der foo klasse oder der std. von int

wobei noch der Zuweisungoperator gebruach wird oder? dereiname von Foo auf Foo zuweisen kann , udn von int auf Foo!!

c= a+b;
"a+b" : operator+ (l,r) dann das ergebenis (ab) und "c=ab" operator=(r).

Hier mein code up-to-date:

class CRoute{

		CTypedPtrArray<CPtrArray,CGroup*> m_paNode;
		int m_i;

	public:
		//Konstruktoren
		CRoute(CGroup *p){
			m_paNode.Add(p);
		}
		CRoute(){
			//Kein Tiefes löschen notwendig da nur Referenzen
			m_paNode.RemoveAll();
		}
		CRoute(CRoute &oScr){
			this->m_paNode.Append(oScr.m_paNode);
		}
		//Destruktor
		~CRoute(){
			m_paNode.RemoveAll();	
		}
		//Anzahl der knoten in einer Route
		inline int GetSize() const{ 
			return m_paNode.GetSize(); 
		}
		//Zugriff auf elemente der Route über Index		
		inline CGroup& CRoute::operator[] (const size_t iIndex){
			return *(m_paNode.ElementAt(iIndex));
		}
		//Route an Route anhängen!
		void CRoute::operator+(CRoute *&p){
			for(int k=0; k< p->GetSize(); k++)
				this->m_paNode.Add(&((*p)[k]));
		}
		//Knoten an Route anhängen
		void CRoute::operator+(CGroup *&p){
			this->m_paNode.Add(p);
		}
		/*Route einer anderne Route zuweisen (Tiefe Kopie nich notwendige, da die Routen nur auf
		Knoten des Baumen zeigen*/
		CRoute& CRoute::operator=(/*const*/ CRoute &oScr){
			if(&oScr== this)
				return *this;
			ASSERT(&oScr!=NULL);
			this->m_paNode.RemoveAll();
			for(int i=0; i< oScr.GetSize(); i++)
				this->m_paNode.Add(&(oScr[i]));
			return *this;
		}
		//Route an vorhanden Route anfügen
		CRoute& CRoute::operator+=(/*const*/ CRouteSector::CRoute &oScr){
			if(&oScr==NULL)
				return *this;
			for(int k=0; k< oScr.GetSize(); k++)
				this->m_paNode.Add(&(oScr[k]));
			return *this;
		}
		//Route an vorhanden Route anfügen
		CRoute& CRoute::operator+=(CGroup &oScr){
			if(&oScr==NULL)
				return *this;
			//for(int k=0; k< oScr.GetSize(); k++)
			this->m_paNode.Add(&oScr);
			return *this;
		}
		//Route ausgeben
		void DEBUG_TRACEOUT(){
			TRACE("Pfad ADR.: %i\n",(int)this);
			for(int i=0; i< this->GetSize(); i++){
				CGroup *p =this->m_paNode.ElementAt(i);
				TRACE("ADR.: %i NODE: %s\n", (int)p, p->GetName()); 
			}
		}
	};

wenn ich const bei den übergabeparameter anfeben (siehe /* const */) kommst aber immer der compilerfehler : d:\Multithreading\multithread\multithread\ProcGraph.h(368): error C2678: binary '[' : no operator found which takes a left-hand operand of type 'const CRouteSector::CRoute' (or there is no acceptable conversion)

CStoll

BorisDieKlinge schrieb:

"1+2" : Denk mal globater + operator der foo klasse oder der std. von int

Fast richtig: "c=1+2;" verwendet (in dieser Reihenfolge) den op+ für int's (eingebaut), den Ctor myNumber(int) und den op= (entweder von dir geschrieben oder implizit angelegt)

wenn ich const bei den übergabeparameter anfeben (siehe /* const */) kommst aber immer der compilerfehler : d:\Multithreading\multithread\multithread\ProcGraph.h(368): error C2678: binary '[' : no operator found which takes a left-hand operand of type 'const CRouteSector::CRoute' (or there is no acceptable conversion)

Vielleicht solltest du auch eine const-Version deines operator[] bereitstellen

BorisDieKlinge

oh man das versteh ich nich mit den const. ich weis zwar was sie bedeuten, aber in welchem zusammmenhang sie hinsichtlich der funltionen udn operatoren stehen müssen ist so ein wirrwarr..

Trozdem noch mal ein Dankeschön zwischendruch an CStoll:) Kommst dir sicher vor wie im kindergarten:)

du meinst:

//Zugriff auf elemente der Route über Index        
        inline const CGroup& CRoute::operator[] (const size_t iIndex){
            return *(m_paNode.ElementAt(iIndex));
        }

??

kann ich dann überhaupt nicht const Objekte der klasse CRoute an die funltionen übergeben?