Zugriff auf einzelne Elemente eines Container-Members

[beselbube]

Wie wärs denn ganz einfach mit einer Add()-Methode, die diese Zusatz-Funktionalität anbietet? Wo einfach ein CWorker übergeben wird.. um die Datenhaltung muss sich ja eigentlich CCompany kümmern.

[brotbernd]

Ich mach meist

class Company
{
public:
    typedef std::vector<Worker>::const_iterator WorkerIterator;

    WorkerIterator FirstWorker() const { return workers.begin(); }
    WorkerIterator LastWorker() const { return workers.end(); }

private:
    vector<Worker> workers;
}

Damit kann man dann z.B.

auto admin = std::find_if(comp.FirstWorker(), comp.LastWorker(), IsAdmin());
if (admin != comp.LastWorker())
  admin->SetGehalt(1€);

Wenn tatsächlich random access nötig ist, gibt es eben einen []operator oder werfende at() methode, die eine WorkerReference zurückgibt.

Ein Klasse wie Company wollte nicht zu einer Gottklasse mutieren, die alles regelt was in einer Firma passiert. Die Aufgabe dieser Klasse wär z.B. alle Mitarbeiter zu speichern und den Zugriff auf sie zu regeln. Die Gehälter nach einer Logik zu verteilen ist Aufgabe von jemand anderem.

[asc]

brotbernd schrieb:

Ich mach meist

Deine Lösung missfällt mir sogar sehr.

Wenn man schon Iteratoren weiterleitet, sollte man dies so machen, das die STL damit etwas anfangen kann. Und die STL erwartet teilweise das die Iteratoren bestimmte Namen haben, ebenso begin() und end().

Ich verwende z.B. das Makro BOOST_FOREACH gerne, das würde bei dir nicht funktionieren, ist aber unter dem aktuellen C++ (Prä-C++0x) eine lesbare Schreibweise (siehe unten).

class Company
{
public:
    // Wenn schon die Iteratoren etc. durchreichen, dann gleich nach STL-Art
    // Wobei man dann auch gleich den Container öffentlich machen kann
    typedef std::vector<Worker>::const_iterator const_iterator;
    const_iterator begin() const { return workers.begin(); }
    const_iterator end() const { return workers.end(); }

private:
    vector<Worker> workers;
};

brotbernd schrieb:

Damit kann man dann z.B.

Leider ist dein Code nicht unter vielen aktuellen Compilern lauffähig. Bei weiten nicht alle Compiler unterstützen schon soviel vom kommenden C++ Standard um auto so wie bei dir zu akzeptieren.

//...
Company::const_iterator admin = std::find_if(comp.begin(), comp.end(), IsAdmin());
//...
BOOST_FOREACH(Worker const & worker, Company) {
  //...
}

[brotbernd]

Mir missfällt deine Lösung (ich hab das früher auch oft so gemacht, inzwischen aber nicht mehr). Company::begin ist absolut nichts sagend. Was anderes wäre vielleicht WorkerContainer::begin, das wär relativ eindeutig, dass ich da einen Worker bekomme. Eine Company könnte außerdem auch noch Buildings haben. Ich möchte dann einen Worker Iterator oder einen Building Iterator haben.
Gute Methodennamen zu wählen ist mir inzwischen (vor allem bei einer Teamgröße > 1) deutlich wichtiger als z.B. BOOST_FOREACH nutzen zu können.

[asc]

brotbernd schrieb:

Mir missfällt deine Lösung...

Ich habe auch nicht behauptet, das sie das Gelbe vom Ei ist, aber sie ist imho dennoch besser als deine Lösung (...Wenn schon die Iteratoren etc. durchreichen...), da sie sich wenigstens an bestehende Schnittstellenvorgaben orientiert.

brotbernd schrieb:

Eine Company könnte außerdem auch noch Buildings haben... Gute Methodennamen zu wählen ist mir inzwischen (vor allem bei einer Teamgröße > 1) deutlich wichtiger als z.B. BOOST_FOREACH nutzen zu können.

Mir sind saubere Bezeichnungen auch lieber (Ich würde hier CompanyWorkers präferieren), aber ebenso wichtig halte ich saubere Schnittstellen (und damit keine die Containerdetails offen legen). Davon abgesehen musst du mir nichts von "Teamgröße > 1" erzählen, ich programmiere beruflich häufiger im Team als einzeln.

Gerade da sind mir Schnittstellen wesentlich wichtiger, weil diese sich möglichst wenig ändern sollten, auch wenn man feststellt das der Container dann doch ungeeignet ist.

[brotbernd]

Diese "bestehende Schnittstellenvorgabe" macht hier aber für meinen Geschmack keinen Sinn. Eine Firma hat keinen Anfang, oder kein erstes Element. Schreibe ich einen STL kompatiblen Container für Worker, würde ich als erstes ein typedef std::vector<Worker> WorkerConatiner ansetzen. Wenn das nicht mehr ausreicht schreibe ich einen eigenen (vorzugsweise generischen) Container, der mir mit begin einen Iterator auf das erste Element (von welchem Typ auch immer) zurückgibt. Hier ging es aber nicht um das Schreiben von STL kompatiblen Containern, sondern um Klassendesign.
Und dass ich was von Teamgröße schreibe heißt nicht, dass ich behaupte du würdest nur allein im Kämmerlein vor dich hinhackern.

[BrauchtManOffensichtlich]

Zunächst mal vielen Dank für die Antworten!

SeppJ schrieb:

Wenn du ohnehin den Behälter zurücklieferst: Warum benutzt du dann nicht dessen Zugriffsoperatoren?

Also so:

foo.GetWorkers()[4] = blabla;  // Ändere 5. Element

Wenn das so funktionieren soll, kann ich den Behälter nicht als konstante Referenz zurückgeben, sondern müsste eine echte Referenz liefern, oder nicht? Und wenn ich das tue, kann ich die Get-Methode gleichzeitig als Setter benutzen. Wäre das nicht fast so als würde ich gleich den Container public machen?

SeppJ schrieb:

Falls CCompany nur einen einzigen Container hält (so dass eindeutig wäre welcher gemeint ist), könnte man auch den Operator [] überladen.

Das liefe im Prinzip auf das heraus, was ich über die Indices gemacht hätte. Nur dass sich ein Aufruf

myCompany[17].GetSalary()

vielleicht schlechter liest als

myCompany.GetWorker(17).GetSalary()

Die Diskussion über die Lesbarkeit solcher Aufrufe führt ihr (brotbernd und asc) ja letzten Endes ebenfalls. Ich muss brotbernd zustimmen, dass es mir wichtig ist, dass ein Aufruf lesbar ist. Bei

myCompany.begin()

würde ich jetzt nicht unbedingt damit rechnen, den ersten Worker vor mir zu haben. Allerdings muss ich auch zugeben, dass ich mir um die stl-Kompatibilität keine Gedanken machen muss, weil wir eigene Containerklassen einsetzen (obwohl ich ein vermeintliches stl-Beispiel gewählt habe.)

brotbernd schrieb:

Ein Klasse wie Company wollte nicht zu einer Gottklasse mutieren, die alles regelt was in einer Firma passiert. Die Aufgabe dieser Klasse wär z.B. alle Mitarbeiter zu speichern und den Zugriff auf sie zu regeln. Die Gehälter nach einer Logik zu verteilen ist Aufgabe von jemand anderem.

Da stimme ich zu, das war auch nicht so gedacht. Aber wenn ich von der Klasse das x-te Element ihres Containers haben möchte, um es zu ändern, sollte mir die Klasse schon eine möglichst komfortable Möglichkeit dafür bieten.

brotbernd schrieb:

Wenn tatsächlich random access nötig ist, gibt es eben einen []operator oder werfende at() methode, die eine WorkerReference zurückgibt.

Und diesen random access hätte ich in meinem Fall eigentlich gerne. Dann müsste ich mir also doch eine Methode GetWorker(int nIndex) schreiben, die mir diese Worker-Referenz über den operator[] oder at() des Containers liefert!?

[SeppJ]

BrauchtManOffensichtlich schrieb:

Wenn das so funktionieren soll, kann ich den Behälter nicht als konstante Referenz zurückgeben, sondern müsste eine echte Referenz liefern, oder nicht? Und wenn ich das tue, kann ich die Get-Methode gleichzeitig als Setter benutzen. Wäre das nicht fast so als würde ich gleich den Container public machen?

Ja. Aber genauso ist es auch, wenn du Getter und Setter für alles anbieten würdest, nur die Schreibweise ist anders. Der formelle Unterschied zwischen exzessiven Gettern und Settern zu public besteht in beiden Fällen weiterhin: Du hast immer noch die Möglichkeit eine Kontrolinstanz einzubauen. Ich muss aber sagen, dass ich kein Fan von Gettern und Settern bin und lieber Klassen schreibe, deren Methoden auch tatsächlich etwas machen. Andere Leute (z.B. du) sehen dies anders.

Das liefe im Prinzip auf das heraus, was ich über die Indices gemacht hätte. Nur dass sich ein Aufruf

myCompany[17].GetSalary()

vielleicht schlechter liest als

myCompany.GetWorker(17).GetSalary()

Genau. Deswegen auch die Einschränkung, dass ich dies nur machen würde, wenn eindeutig klar ist, dass die Worker gemeint sind. Bei diesem konkreten Beispiel stehe ich auf der Seite von brotbernd, dass dies nicht gegeben ist. Ein Unterschied wäre es beispielsweise, wenn du eine Klasse hättest, die einen internen Container wrappt. Das sähe syntaktisch fast genauso aus wie deine CCompany, aber es wäre semantisch etwas völlig anderes.

[BrauchtManOffensichtlich]

SeppJ schrieb:

Ich muss aber sagen, dass ich kein Fan von Gettern und Settern bin und lieber Klassen schreibe, deren Methoden auch tatsächlich etwas machen. Andere Leute (z.B. du) sehen dies anders.

Das hängt meiner Meinung nach stark von der jeweiligen Klasse ab. Klassen, die ausschließlich Getter und Setter enthalten, halte ich selten für sinnvoll. Ganz ohne Get/Set komme ich aber ebenfalls selten aus. Eine gesunde Mischung aus Methoden, die mit den Daten arbeiten und welchen, die einem den Zugriff auf die Daten erlauben, ist es meistens

Danke jedenfalls nochmal für die Antwort. Ich bin mir zwar immer noch nicht ganz sicher, welche Variante mir am besten gefällt, aber immerhin habe ich ein paar Anregungen bekommen.

[Simon2]

brotbernd schrieb:

...Company::begin ist absolut nichts sagend. ... Eine Company könnte außerdem auch noch Buildings haben. Ich möchte dann einen Worker Iterator oder einen Building Iterator haben....

Ich verstehe (und sehe) dein Problem.
... aber auch das von Sepp:
Liegt es aber nicht daran, dass beide Ansätze (begin()/end() & FirstWorker()/LastWorker()) nicht sauber abstrahieren?

* begin()/end() reduziert implizit CCompany auf einen WorkerContainer (wie du schon festgestellt hast)
* FirstWorker()/LastWorker() schränkt den WorkerContainer in seiner Flexibilität unangemessen ein.

Da hat der Ansatz von "BrauchtManOffensichtlich" (expliziter zugriff via GetWorkers()) Stärken. Der könnte auch einen non-const-Referenz zurückgeben und es wäre alles klar:
- durch ein GetWorkers() will ich nur den "Containerteil von CCompany" und
- der zurückgegebene Container kann eine möglichst große Flexibilität bieten - unabhängig von der Schnittstelle von CCompany.

Hakelig wird's natürlich, wenn intern ein Container verwendet werden soll, der diese Flexibilität nicht bietet. Dann muss man ein entsprechend aufwendiges Interface bauen.... (aber das muss man dann gegen die Vorteile des Containers abwägen).

Gruß,

Simon2.

[brotbernd]

Simon2 schrieb:

Liegt es aber nicht daran, dass beide Ansätze (begin()/end() & FirstWorker()/LastWorker()) nicht sauber abstrahieren?
[...]
Da hat der Ansatz von "BrauchtManOffensichtlich" (expliziter zugriff via GetWorkers()) Stärken. Der könnte auch einen non-const-Referenz zurückgeben und es wäre alles klar:[...]

Warum jetzt das Zurückgeben des ganzen Containers eine höhere Abstraktion darstellt versteh ich nicht. Das Iteratorkonzept abstrahiert doch den Zugriff auf eine Datensammlung, egal worin die Daten da eigentlich stecken, der Zugriff sieht immer gleich aus. Im Falle der STL ist das zwar eine "Kompilierzeitabstraktion" aber das spielt in diesem (und den meisten) Fällen doch keine Rolle.
Ich habe auch nur durch die FirstX / LastX Schnittstelle eine hohe Flexibilität. Ich kann mit distance , advance oder + operatoren auch nicht member Funktionen dazudichten, die mir auch Worker an Stelle x zurückgeben. Unabhängig vom eigentlich Container (die komplexität ist nur entsprechend höher bei nicht random access containern). Ich muss allerdings sagen, dass ich i.d.R. dann auch eher eine WorkerAt(size_t pos) Methode anbiete.

Generell sollte man halt die öffentliche Schnittstelle minimal halten. Getter und Setter gibt es nur wenn sie wirklich gebraucht werden. Sowieso schreibt man nur Code, wenn man ihn braucht (natürlich nicht wer Bibliotheken schreibt).
Wie notwendig Getter/Setter sind ist sehr unterschiedlich. Z.B. sind Klassen wie Company in GUI Anwendungen viel eher mit Gettern ausgestattet, da es doch recht viel Aufwand ist z.B. eine Schnittstelle zu einem List View so zu abstrahieren, dass die Worker da auf wunderlicher weise drin landen. Ich zumindest mach es mir da lieber einfach (bin aber auch nicht so der erfahrende GUIler).

[antialias]

Wenn das so funktionieren soll, kann ich den Behälter nicht als konstante Referenz zurückgeben, sondern müsste eine echte Referenz liefern, oder nicht? Und wenn ich das tue, kann ich die Get-Methode gleichzeitig als Setter benutzen. Wäre das nicht fast so als würde ich gleich den Container public machen?

Machst du ja auch praktisch über deine getter und setter - denn du übergibst damit dem Benutzer die volle Kontrolle über deinen vector.

Wie SeppJ schon schreibt: Geschmacksfrage. icbin auch kein Fan von getter/setter. Die Gaukeln nur Sicherheit vor.

[Simon2]

brotbernd schrieb:

...
Warum jetzt das Zurückgeben des ganzen Containers eine höhere Abstraktion darstellt versteh ich nicht. ...

Einfach weil du die Schnittstelle das Containers von der der Fachklasse (CCompany) trennst.
Sonst stößt du z.B. schnell an Grenzen, wenn die Anforderungen an den Container sehr umfangreich sind oder sich wandeln.
Z.B.:
- das Iteratorkonzept ist sehr mühsam, wenn man Random Access (z.B. mit operator[]) braucht)
- Wenn du versuchst, alle möglichen Containereigenschaften deiner Fachklasse "beizubringen", brichst du einerseits mit deinem eigenen (sehr sinnvollen Prinzip) der "Minimalen Schnittstelle" und kommst andererseits schnell in Konsistenzsschwierigkeiten (wenn "auf Vorrat" Funktionen eingebaut werden, die gar nicht zum verwendeten Container passen).
- Wird in Version 2 fachlich plötzlich eine "Abbildungsmimik" (für die sich z.B. eine map anbietet) sinnvoll, muss CCompany massiv umgestrickt werden

Ich weiß gar nicht, warum ein getter immer nur primitive Datentypen zurückgeben dürfen soll ... ein Container ist doch auch ein passabler Rückgabewert.
Es käme ja auch keiner auf den Gedanken, einen string nur char-weise via Iterator zugreifbar zu machen.

BrauchtManOffensichtlich schrieb:

...Wenn das so funktionieren soll, kann ich den Behälter nicht als konstante Referenz zurückgeben, sondern müsste eine echte Referenz liefern, oder nicht? Und wenn ich das tue, kann ich die Get-Methode gleichzeitig als Setter benutzen. Wäre das nicht fast so als würde ich gleich den Container public machen? ....

Das Problem hat aber nichts mit Containern zu tun, sondern stellt sich bei jedem Member. Es ist lediglich die (vermutlich aus C geerbte) verf***te und wirklichkeitsfremde "Tuning-Paranoia", die bei Vielen alle roten Lampen aufleuchten lässt, sobald ein nichtprimitiver Datentyp über den Horizont blickt.

Gruß,

Simon2.

[brotbernd]

Simon2 schrieb:

- Wird in Version 2 fachlich plötzlich eine "Abbildungsmimik" (für die sich z.B. eine map anbietet) sinnvoll, muss CCompany massiv umgestrickt werden.

Schlimmer: Gibts du eine Referenz auf den ganzen Container raus, benutzt die ganze Welt die Schnittstelle von vector<Worker>. Änderst du vector<Worker> in set<Worker> musst du die ganze Welt ändern, denn Du hast keine Kontrolle darüber was die ganze Welt von der Schnittstelle der Container verwenden.
Nur weil du nur eine Methode schreibst um den Container rauszurücken, ist das keine minimale Schnittstelle. Die Schnittstelle ist damit mindestens so groß wie die des Containers. D.h. änderst du den Container, änderst du die Schnittstelle.
Der Vergleich mit Strings ist zwar ganz interessant aber Strings würde ich auch schon fast eher als primitive Typen ansehen.

[BrauchtManOffensichtlich]

Freut mich, dass ich hier eine muntere Diskussion angeregt habe. Interessant finde ich, dass ich jetzt in anderen Worten noch einmal genau das lese, worum es mir eigentlich die ganze Zeit ging.

brotbernd schrieb:

Gibts du eine Referenz auf den ganzen Container raus, benutzt die ganze Welt die Schnittstelle von vector<Worker>. Änderst du vector<Worker> in set<Worker> musst du die ganze Welt ändern, denn Du hast keine Kontrolle darüber was die ganze Welt von der Schnittstelle der Container verwenden.

Da ich die Möglichkeit haben wollte, mir den kompletten Container geben zu lassen und auch setzen zu können, habe ich das über ein klassisches Get/Set gemacht wie in meinem ersten Beitrag beschrieben.

BrauchtManOffensichtlich schrieb:

const vector& GetWorkers() const {return m_Workers;}
void          SetWorkers(const vector& workers) {m_Workers = workers;}

Da ich aber auch die Möglichkeit haben wollte ein einzelnes Containerelement zu ändern, hatte ich mit der obigen Schnittstelle (wegen der konstanten Referenz) ein Problem und habe Bedenken geäußert, den Container per (nicht konstanter) Referenz zurückzuliefern, wobei mir einer der Gründe erst durch "brotbernds" Beitrag (siehe obiges Zitat) klar geworden ist

Simon2 schrieb:

Das Problem hat aber nichts mit Containern zu tun, sondern stellt sich bei jedem Member. Es ist lediglich die (vermutlich aus C geerbte) verf***te und wirklichkeitsfremde "Tuning-Paranoia", die bei Vielen alle roten Lampen aufleuchten lässt, sobald ein nichtprimitiver Datentyp über den Horizont blickt.

Dass das auch für nichtprimitive Datentypen gilt, ist mir natürlich klar, aber ich finde brotbernds Argument zieht schon irgendwie

Ich bin mir inzwischen jedenfalls recht sicher, dass ich es über einen Ansatz der folgenden Art machen werde.

CWorker& GetWorker(int nIndex){return m_Workers[nIndex];} // vielleicht prüfe ich den Index sogar noch vorher ;)

Letzte Bedenken können aber gerne noch einmal geäußert werden

[brotbernd]

Eine GetWorker ist schon ok. Benutze size_t pos als Argument und den Range check bekommst du durch return m_Workers.at(pos);
Und wie du vielleicht schon mitbekommen hast, sagt kein Schwein mehr CWorker sondern Worker. Aber das ist ja eher Geschmackssache.

[Simon2]

brotbernd schrieb:

...
Änderst du vector<Worker> in set<Worker> musst du die ganze Welt ändern, ...

Keineswegs. Ich kann durchaus weiterhin einen vector rausgeben - auch wenn ich intern die Daten anders verwalte.
Das wird mit einem non-const-Ref-vector natürlich "schwieriger" ... aber wenn man sich dagegen absichern will, nimmt man eben die "getter/setter"-Variante oder einen Container, der das Fachobjekt über Änderungen informiert.

brotbernd schrieb:

...denn Du hast keine Kontrolle darüber was die ganze Welt von der Schnittstelle der Container verwenden....

doch. Es ist eben genau die Schnittstelle des Containers - und zwar die ganze, die dann auch genauso funktioniert, wie der Anwender es erwartet.

Wie schon gesagt: Das ist exakt genauso wie bei primitiven Datentypen: Du kämst doch auch nicht auf die Idee, für ein int- oder ein string-Member alle möglichen (bzw. alle, die dir momentan einfallen) Operationen ans Fachobjekt abzugeben.
Entweder verlässt man sich drauf, dass es seinen Typen nicht ändert und man gibt einen non-const-Ref raus.
Oder man will sich nicht darauf verlassen => getter/setter, die ggf. auf interne Datenstrukturen abgebildet werden.

brotbernd schrieb:

...aber Strings würde ich auch schon fast eher als primitive Typen ansehen.

Und warum vector nicht?

Ich behaupte aber mal, dass das direkte Bearbeiten eines internen Containers eher selten ist (bei fachlich hinreichend abstrakten Klassen). So würde ich erwarten, dass CCompany

• eher eine Funktion "stelle_MA_ein()" anbietet (mit einer ganzen Menge fachlicher Abläufe)
• als zu erwarten, dass der Anwender ein vector<MA>push_back() macht.

Die Herausgabe von Containern (Listen, ...) kenne ich eigentlich eher aus so Sachen wie "Datenextraktion" (z.B. zur Anzeige).

Gruß,

Simon2.

[brotbernd]

Simon2 schrieb:

brotbernd schrieb:

...
Änderst du vector<Worker> in set<Worker> musst du die ganze Welt ändern, ...

Keineswegs. Ich kann durchaus weiterhin einen vector rausgeben - auch wenn ich intern die Daten anders verwalte.
Das wird mit einem non-const-Ref-vector natürlich "schwieriger" ... aber wenn man sich dagegen absichern will, nimmt man eben die "getter/setter"-Variante oder einen Container, der das Fachobjekt über Änderungen informiert.

Was heißt hier keineswegs. Dass der vector durch ein set ersetzt wird, ist mein Szenario das ich beschrieben habe. Die ganze Welt hat vorher die Schnittstelle des vectors benutzt, den du bisher immer rausgegeben hast. Jetzt willst du die Worker aber in einem set speichern. Was machst du nun? Eine Referenz auf das set herausgeben? Dann musst du in der ganzen Welt ändern, denn alle benutzen das Interface von vector und das stimmt nicht überein mit set. Oder möchtest du etwa die Daten aus dem set in einen vector kopieren und den dann rausgeben?

brotbernd schrieb:

...aber Strings würde ich auch schon fast eher als primitive Typen ansehen.

Und warum vector nicht?

Für Strings interessiert man sich i.d.R. im ganzen. Für vectoren nicht (immer). Da interessieren mich meistens eher die einzelnen Elemente.

[oo_O]

brotbernd schrieb:

Eine GetWorker ist schon ok. Benutze size_t pos als Argument und den Range check bekommst du durch return m_Workers.at(pos);
Und wie du vielleicht schon mitbekommen hast, sagt kein Schwein mehr CWorker sondern Worker. Aber das ist ja eher Geschmackssache.

Wenn man langsamen Code will, OK.

[Nexus]

Dieser tolle Range-Check bei at() , wie oft braucht man den tatsächlich? Wie viel öfter missbraucht man ihn, um Logikfehler mit catch statt assert zu fangen?