Entfernen eines bestimmten Elements aus einem Vector

[vell0cet]

Hallo zusammen!!

Ich bin C++-Anfänger und komme eigentlich aus der Java-Ecke. Seit ein paar Tagen arbeite ich mich im Rahmen eines Projekts in C++ ein.

Bevor ich meine Frage erläutere möchte ich sagen, dass ich Google, die Suchfunktion und die gängigen Anfängerfragen bereits durchforstet habe. Leider konnte ich keine erschöpfende Antwort auf meine (wahrscheinlich banale) Frage finden.

Ich hoffe, dass mir hier jemand helfen kann.

Es geht um Folgendes: Ich möchte innerhalb einer Klasse "Source" verschiedene Objekte der Klasse "Device" in einem Vector verwalten. Der Vector soll als private Instanzvariable der Klasse "Source" vorliegen. Per addDevice(), getDevice() und removeDevice() möchte ich auf dem Vector gekapselt operieren. Das Einfügen und Auslesen aus dem Vector funktioniert schonmal ganz gut. Nur das Löschen bereitet mir Probleme. Ich bekomme Fehlermeldungen vom Compiler, wenn ich folgende Funktion benutze:

devices.erase(remove(devices.begin(), devices.end(), dev), devices.end());

Zum tieferen Verständnis liste ich am besten einmal meinen (verkürzten) Code. Ich vermute, dass ich mit Pointern arbeiten muss und habe da auch schon Verschiedenes ausprobiert. In den Listings habe ich aber meine Ausgangslösung gepostet.

Main.cpp

#include <iostream>
#include <string>
#include "Source.h"

using namespace std;

int main() {
	Device d1 = Device("Device 1");
	Device d2 = Device("Device 2");

	Source src = Source();
	src.addDevice(d1);
	src.addDevice(d2);

	cout << src.getDevice(0).label << endl;
	cout << src.getDevice(1).label << endl;
	cin.get();
}

Source.h

#pragma once
#include <string>
#include <vector>
#include "Device.h"

using namespace std;

class Source {
public:
	void addDevice(Device dev);
	void removeDevice(Device dev);
	Device getDevice(int pos);

private:
	vector<Device> devices;
};

Source.cpp

#include <algorithm>
#include "Source.h"

void Source::addDevice(Device dev) {
	devices.push_back(dev);
}

void Source::removeDevice(Device dev) {
	devices.erase(remove(devices.begin(), devices.end(), dev), devices.end());
}

Device Source::getDevice(int pos) {
	return devices.at(pos);
}

Device.h

#pragma once
#include <string>

using namespace std;

class Device {
public:
	string label;

	Device(void);
	Device(string lab);
};

Device.cpp

#include "Device.h"

Device::Device() {
}

Device::Device(string lab) {
	label = lab;
}

Falls es nötig ist, kann ich auch noch die Fehler-Liste hier posten, aber vielleicht hat ja jemand eine schnelle Antwort und findet den Fehler.

[drakon]

Du brauchst da den Iterator des Elements, dass du löschen willst.

http://www.cplusplus.com/reference/stl/vector/erase.html

Wenn du lediglich das Objekt hast, kannst du ja einfach danach suchen und dann den Iterator übergeben.

void Source::removeDevice(Device dev) {
    std::vector<Device>::iterator it = devices.begin();

    if ( it != devices.end () )
        devices.erase ( it );
}

Das nächste mal wäre aber die Fehlermeldung sicher auch hilfreich.
(Also meist reicht einfach die erste Meldung, weil alles andere normalerweise folgefehler sind und somit selbst verschwinden.)

[vell0cet]

Ich danke Dir für die schnelle Antwort. Ich werde es später ausprobieren.

[Nexus]

Da du aus Java kommst... Du willst ja schon, dass die Device -Einträge kopiert werden und nicht nur Verweise im std::vector gespeichert sind, oder? Denn in diesem Fall müsstest du Zeiger einsetzen.

Und hier

Device::Device(string lab)

solltest du besser eine Const-Referenz einsetzen, da sonst jedes Mal der ganze std::string kopiert wird.

Device::Device(const string& lab)

Das gilt natürlich auch für die Übergaben bei addDevice() und removeDevice() .

Dann ist die folgende Zeile auch umständlich, weil zuerst ein temporäres Objekt erstellt und dieses dann in das eigentliche kopiert wird:

Device d2 = Device("Device 2");

Besser, du initialisierst es gleich:

Device d2("Device 2");

Und als letztes ist using namespace in Headern zu vermeiden, da du dann in allen Dateien, wo du den Header einbindest, den gesamten Namensraum offenlegst (was nicht immer erwünscht ist und zu Namenskonflikten führen kann).

[drakon]

Nexus schrieb:

Da du aus Java kommst... Du willst ja schon, dass die Device -Einträge kopiert werden und nicht nur Verweise im std::vector gespeichert sind, oder? Denn in diesem Fall müsstest du Zeiger einsetzen.

Die werden ja kopiert..

Dann ist die folgende Zeile auch umständlich, weil zuerst ein temporäres Objekt erstellt und dieses dann in das eigentliche kopiert wird:

Das glaube ich kaum.. Das wird i.d.R wegoptimiert.

Allerdings wäre der Einsatz von Initialierungslisten sicher auch angebracht, anstatt die Zuweisugn im c-tor.

Device::Device()
:label("") {
}

Device::Device(string lab)
:label(lab) {
}

[Kritiker]

Nexus schrieb:

Dann ist die folgende Zeile auch umständlich, weil zuerst ein temporäres Objekt erstellt und dieses dann in das eigentliche kopiert wird:

Sicher?

[Kritiker]

Ups, war schon einer schneller.

[drakon]

Kritiker schrieb:

Ups, war schon einer schneller.

Bist halt nich der einzig kritische hier.

[Kritiker]

drakon schrieb:

Bist halt nich der einzig kritische hier.

Hoffe du verzeihst mir jetzt auch eine kleine Kritik an deinem ersten Beitrag?!?

Was vell0cet da versucht ist eingentlich schon OK und er bekommt ja auch den Iterator für das(die) zu löschende(n) Element(e). So muss es eigentlich auch gemacht werden.

In meinen Augen liegt es einfach daran, dass er den == Operator für die Klasse Devices vergessen hat.

Aber, gut vielleicht hören wir ja noch mehr.

[drakon]

Kein Problem..

Ich habe den Code gar nicht so intensiv angeschaut gehabt und gesehen, dass er nur ein Objekt löschen will. Und als er etwas von Zeiger geredet hat, habe ich es den Code gar nicht mehr grossartig angeschaut..

Ich denke mal, dass die Fehlermeldung hier mehr ausgesagt hätte. :p - Darum immer posten.
Hier liegts tatsächlich am fehlenden ==-Operator..

[Nexus]

drakon schrieb:

Die werden ja kopiert..

Eben deshalb frage ich ja. In Java würde das Objekt soviel ich weiss nicht kopiert.

drakon schrieb:

Das glaube ich kaum.. Das wird i.d.R wegoptimiert.

Aha, sich einfach mal von vornherein drauf verlassen, dass der Compiler super wegoptimieren kann, anstatt einen gleichwertigen Fall zu nehmen, bei dem nichts optimiert werden muss?

Es ist vorgesehen, dass in dem Fall ein temporäres Objekt erzeugt wird. Gewisse Umstände verhindern auch ein Wegoptimieren. Bau zum Beispiel mal ein Logging in den Konstruktor ein; ich will sehen, ob man von der erstellten Klasse nichts mitbekommt.

[drakon]

Du willst ja schon, dass die Device-Einträge kopiert werden und nicht nur Verweise im std::vector gespeichert sind, oder? Denn in diesem Fall müsstest du Zeiger einsetzen.

Wenn da anstatt "in diesem Fall" "andernfalls" gestanden wäre, hätte ichs verstanden.

Nexus schrieb:

Es ist vorgesehen, dass in dem Fall ein temporäres Objekt erzeugt wird. Gewisse Umstände verhindern auch ein Wegoptimieren. Bau zum Beispiel mal ein Logging in den Konstruktor ein; ich will sehen, ob man von der erstellten Klasse nichts mitbekommt.

Das ist afaik eine sehr gängige und übliche Optimierung.

struct foo
{
 foo ()
 {
  std::cout << "ctor-foo" << std::endl;
 }
};

int main ()
{
 foo f = foo ();
}

Ausgabe:

ctor-foo

[Nexus]

drakon schrieb:

Wenn da anstatt "in diesem Fall" "andernfalls" gestanden wäre, hätte ichs verstanden.

Stimmt, war etwas missverständlich. Ich bezog mich mit "diesem" auf das letzgenannte (hier die Verweise).

drakon schrieb:

Das ist afaik eine sehr gängige und übliche Optimierung.

Das dachte ich mir fast, aber dass das Verhalten des Programms verändert wurde, wusste ich nicht (ich kann deine Ausgabe bestätigen). Denn eigentlich müsste streng genommen ein temporäres Objekt erzeugt werden, oder? Zumindest dachte ich, es wäre nicht erlaubt, dass die Optimierung das Programmverhalten ändert...

[Kritiker]

Nexus schrieb:

Denn eigentlich müsste streng genommen ein temporäres Objekt erzeugt werden, oder?

Ja, so ist es. Ohne Optimierung würde ein Konstruktor und anschließend der Copy-Konstruktor aufgerufen. Wenn du den Copy-Konstruktor private deklarierst, dann scheitert auch die ganze Geschichte, im Gegensatz zur Direct-Initialisierung wo ja von Beginn an nur der Konstruktor aufgerufen wird.

Nexus schrieb:

Zumindest dachte ich, es wäre nicht erlaubt, dass die Optimierung das Programmverhalten ändert...

In diesem Fall ist es erlaubt, genauso wie bei der vielgerühmten "return value optimization", wo ja auch der Copy-Konstruktor umgangen wird.

Dennoch, obwohl unter Strich (meist) dasselbe herauskommt ist die Version

Device d2("Device 2");

die empfohlene Variante, daher war dein Tipp meiner Meinung nach grundsätzlich berechtigt

[Nexus]

Hmm. Merkwürdige Sache. Der Kopierkonstruktor scheint tatsächlich nicht aufgerufen zu werden, ich hab nochmal geschaut...

Vor allem ist die Direktinitialisierung die einzige Möglichkeit, wenn man zum Beispiel nicht-kopierbare Objekte hat.

[Kritiker]

Nexus schrieb:

Hmm. Merkwürdige Sache.

Wieso ist das merkwürdig? Wir hatten uns doch geeinigt :), dass das Ding wegoptimiert wird.

Dass er aber grundsätzlich gebraucht wird, also tatsächlich erst durch Optimierung wegfällt zeigt die private-Version. Oder verstehe ich dich jetzt falsch?

[drakon]

Kritiker schrieb:

Dennoch, obwohl unter Strich (meist) dasselbe herauskommt ist die Version

Device d2("Device 2");

die empfohlene Variante, daher war dein Tipp meiner Meinung nach grundsätzlich berechtigt

Dann müsstest du aber auch:

int i (2);

machen.

Ich Ich finds in manchen Situationen einfach lesbarer und schneller verständlich, wenn ich das mit dem Zuweisungsoperator (welcher hier keiner ist ) zu schreiben.

Klar, wenn man einen Konstruktor verschieden vom Kopierkonstruktor hat, dann hat man in diesem Falle ein Problem..

[Kritiker]

Es geht bei der Empfehlung natürlich um die Initialisierung mit Konstruktoraufruf. Bei int usw. gibt es die Unterschiede ja nicht.

Was jetzt übersichtlicher ist liegt selbstverständlich im Auge des Betrachters, da will ich keine Wertung abgeben.

Aber, klar ist auch, dass die Variante

int i (2);
Device d2("Device 2");
// ... usw. usw.

immer funktioniert, während man mit

int i = 2;
Device d2 = Device("Device 2");
Device d2 = "Device 2";
// ... usw. usw.

durchaus straucheln kann und gezwungen ist die erstgenannte Variante zu wählen, auch wenn man sie als unübersichtlich empfindet.

Nun gut, das sind ja alles mehr oder weniger unwichtige Details, soll jeder machen wie es ihm beliebt ....

[drakon]

Kritiker schrieb:

immer funktioniert, während man mit

Wirklich immer?

foo g = foo ( foo ( foo () ) );
//vs
foo f ( foo( foo ( foo () ) ) );

Über Sinn und Unsinn von solchen Konstrukten sei wo anders diskutiert..

[Kritiker]

drakon schrieb:

Wirklich immer?

Die Entsprechung zu

foo g = foo ( foo ( foo () ) );

ist nicht die Funktionsdeklaration

foo f ( foo( foo ( foo () ) ) );

sondern der Aufruf des Standardkonstruktors

foo f;

Du könntest jetzt anführen, dass der Copy-Konstruktor wichtige Änderungen am Objekt vornimmt und deshalb genau 3-mal hintereinander aufgerufen werden muss.

Dem halte ich entgegen, dass du

1. den mehrfachen Aufruf nicht garantieren kannst (Wenn doch, will ich wissen wie du es gemacht hast!!!) und

2. wäre ja dann auch nicht garantiert, dass

foo g = foo();
foo g;

oder

foo dummy;
foo g = dummy;
foo g(dummy);

jeweils dasselbe Ergebnis für g liefert.

Du hättest sozusagen ernsthafte Problem zu erklären warum

foo g = foo ( foo ( foo () ) );

nicht gleichbedeutend ist mit

foo f;

Aber klar, da ich "immer" gesagt habe, hast du meine Aussage sozusagen mit einem chaotischen Copy-Konstruktor "logisch" widerlegt.

Genieße die Genugtuung!