[ERLEDIGT] normaler iterator und const iterator

darkfate

Hallo Leute,

ich versuche diese Beiden Funktionen möglichst elegant zu verschmelzen:

Die Typedefs:

typedef std::map<UINT,UTM, std::less<UINT>>::iterator UTM_ITERATOR;
typedef std::map<UINT,UTM, std::less<UINT>>::const_iterator UTM_CONST_ITERATOR;

Die Suchfunktion liefert die Position der Koordinate:

inline UTM_CONST_ITERATOR GEOTopology::find_coord_by_id(UINT searched_id) const{

	UTM_CONST_ITERATOR utmconstit = coords.find(searched_id);
	return utmconstit;
}

Die Löschfunktion soll diese Anhand der gefundenen Position löschen.

void GEOTopology::del_coord_by_id(UINT useless_id){

	UTM_ITERATOR tmpcoopos = find_coord_by_id(useless_id);
	(coords.end() != tmpcoopos) ? coords.erase(tmpcoopos) : exit(1);

	UINT knotdelret;
	knotdelret = knots.erase(useless_id);
	if(!knotdelret) exit(7);

	del_edges_by_knot_id(useless_id);
}

Mir ist klar das sich der const int mit dem normalen int nicht vertragen. Gibt es da einen intelligenten Umweg ohne einen Wildsaucode zu produzieren?

PS: bitte nicht über die exits spotten. Wenn der Code funktioniert fliegen sie auch raus

Was hintert Dich denn daran,

UTM_ITERATOR tmpcoopos = find_coord_by_id(useless_id);

durch

UTM_ITERATOR tmpcoopos = coords.find(useless_id);

in del_coord_by_id zu ersetzten?

Das mit den Bezeichnern würde ich mir auch abgewöhnen. Bezeichner ohne kleine Buchstaben werden typischerweise nur für Makros benutzt.

brotbernd

del_coord_by_id ist nicht const. Du kannst dort also auch eine nicht konstante Version von find_coord_by_id aufrufen, die einen nicht konstanten Iterator zurückgibt.

darkfate

krümelkacker schrieb:

Was hintert Dich denn daran,
UTM_ITERATOR tmpcoopos = find_coord_by_id(useless_id);
durch
UTM_ITERATOR tmpcoopos = coords.find(useless_id);
in del_coord_by_id zu ersetzten?

Sollte man nicht wegen der Datenkapselung auf die Daten nur mit get und set Funktionen zugreifen? Oder ist es innerhalb der Klasse ok?

krümelkacker schrieb:

Das mit den Bezeichnern würde ich mir auch abgewöhnen. Bezeichner ohne kleine Buchstaben werden typischerweise nur für Makros benutzt.

In welcher Art sollte man sie sonst von den üblichen typen hervorheben?

darkfate

brotbernd schrieb:

del_coord_by_id ist nicht const. Du kannst dort also auch eine nicht konstante Version von find_coord_by_id aufrufen, die einen nicht konstanten Iterator zurückgibt.

Dessen war ich mir bewusst.

brotbernd

darkfate schrieb:

Dessen war ich mir bewusst.

hm, dann hab ich die Frage wohl nicht verstanden.
Es geht dir doch um die Zeile:

UTM_ITERATOR tmpcoopos = find_coord_by_id(useless_id);

find_coord_by_id gibt einen const_iterator zurück, den kannst du natürlich nicht einem iterator zuweisen. Also brauchst du eine nicht konstante find_coord_by_id, oder du rufst einfach

UTM_ITERATOR tmpcoopos = coords.find(useless_id);

auf.

darkfate

brotbernd schrieb:

...

Der Code so wie er da steht ist optimierungsbedürftig, deswegen wollte ich fragen wie man so etwas am besten löst.

Soweit es meinem C++ Wissen entspricht, sollte man auf Klassenelemente wie z.b. in diesem Fall:

class GEOTopology{

	private:
		.....
		std::map<UINT, UTM,  std::less<UINT>> coords;
		std::map<UINT, Knot, std::less<UINT>> knots;
		std::map<UINT, Edge, std::less<UINT>> edges;
		.....

	public:
		.... // get und set methoden
};

am besten über get und set Methoden zugreifen um mögliche Fehler abzufangen.
Methoden die nichts am Datenbestand verändern sollen als const deklariert werden.

Die Funktion find_.... verändert ja keine Daten also wollte ich sie als const deklarieren und die const iteratoren verwenden.

Dann kann ich aber logischerweise nicht die methode del_... verwenden weil sie keine const ist ...

Ich bin gerade am überlegen ob es nicht besser wäre einfach die Integer Position über die Methode zurückzugeben.. wäre ein fauler Kompromiss.

brotbernd

Du hast immer noch nicht gesagt, was gegen eine nicht konstante find_coord_by_id(useless_id) spricht. Die muss ja nicht öffentlich sein.
Oder halt einfach das direkte
UTM_ITERATOR tmpcoopos = coords.find(useless_id);

Nexus

darkfate schrieb:

PS: bitte nicht über die exits spotten. Wenn der Code funktioniert fliegen sie auch raus

Dafür würde ich eher assert nehmen, dann bist du auch gleich bei der Fehlerstelle.

darkfate schrieb:

typedef std::map<UINT,UTM, std::less<UINT>>::iterator UTM_ITERATOR;

Das dritte Template-Argument std::less<UINT> ist nicht nötig, da std::less<T> dem Defaultfunktor bei assoziativen STL-Containern entspricht. Zudem solltest du für portablen Code die >> trennen und > > schreiben.

darkfate schrieb:

Sollte man nicht wegen der Datenkapselung auf die Daten nur mit get und set Funktionen zugreifen? Oder ist es innerhalb der Klasse ok?

Für die öffentliche Schnittstelle ist das okay, aber innerhalb der Klasse gewinnst du durch die Indirektion über find_coord_by_id nichts. Zumindest solange du nicht planst, plötzlich mit einer speziellen Semantik zu suchen oder so. Ich würde es nicht unnötig kompliziert machen, sondern einfach

UTM_ITERATOR tmpcoopos = coords.find(useless_id);

schreiben.

darkfate schrieb:

In welcher Art sollte man sie sonst von den üblichen typen hervorheben?

Warum überhaupt hervorheben?

darkfate

brotbernd schrieb:

...

Hast recht, im Prinzip spricht auch nichts dagegen.

Nexus schrieb:

...

Feine Tipps, damit kann ich weitermachen.

Danke an alle Helfenden.