Wie erhalte ich auf unordered_map mit unique_ptr in einer Klasse Vollzugriff (auf iterator)

MrSpoocy

Ich habe folgende Klasse (teilweise etwas gekürzt)

class ObjectList
{		
private:
	typedef std::unique_ptr<Item> ItemUPtr;
	typedef std::unordered_map<short, ItemUPtr> ItemList;
	ItemList list;

public:
	ObjectList();

	void add(ItemUPtr &item)
	{
		this->list[item->index] = std::move(item);
	}

	Item* getByName(const tstring &name)
	{
		for (auto const &it : this->list)
		{
			if (it.second->getName().compare(name) == 0)
			{
				return it.second.get();
			}
		}
		return NULL;
	}
	Item* getByIndex(const short index)
	{
		auto const it = this->list.find(index);
		if (it != this->list.end())
		{
			return it->second.get();
		}

		return NULL;
	}

	bool remove(const Item *item);
	bool removeByName(const tstring &name);
	bool removeByIndex(const short index);

	Item* find();
	void clear();
	bool empty();

	// Diese Funktion geht seit der umstellung von Item* zu unique_ptr<Item> nicht mehr
	ItemList getList();
};

In der bin ich bei der map auf unique_ptr<Item> umgestiegen da oft Probleme wegen der Lebensdauer etc gab. Das klappt jetzt auch alles ganz gut. Nur eine Funktion macht jetzt Probleme.

for (auto &it : Obj->pointerAufObjectList->getList())
{
	auto item = it.second.get();
}

Fehler 1 error C2280: 'std::unique_ptr<abc::Item,std::default_delete<_Ty>>::unique_ptr(const std::unique_ptr<_Ty,std::default_delete<_Ty>> &)' : Es wurde versucht, auf eine gelöschte Funktion zu verweisen C:\Program Files (x86)\Microsoft Visual Studio 12.0\VC\include\xmemory0 592 1 test

Es ist mir auch klar das es nicht gehen darf. Weil ja ObjectList der Besitzer ist. Aber es muss doch eine Möglichkeit geben irgendwie in einer for schleife die einzelnen Pointer zu den Elemente zu erhalten.

Vorweg zu der Frage warum ich bei den get Funktionen den Pointer und nicht ItemUPtr& zurück gebe... ich muss prüfen können ob wirklich ein Element zurückgegeben wurde. Auch wenn eben kein passendes Element gefunden wurde.

Danke Spoocy

Deine getList Funktion gibt eine Kopie der intern gespeicherten Liste zurück. Da die Liste aber unique_ptr enthält, kann keine Kopie der Liste angelegt werden (unique_ptr kann man auch nicht kopieren). Du kannst aber eine konstante Referenz auf die Liste zurück geben:

const ItemList& getList()
{
  return list;
}

Deine Funktion void add(ItemUPtr &item) ist im übrigen ungünstig implementiert. Der Parameter ist eine normale Referenz und dann moved die Funktion den Parameter. Dies ist von außen aber nicht ersichtlich und von einer add Funktion erwarte ich eigentlich, dass diese den Parameter nicht ändert. Besser sollte die Funktionen eine RValue Referenz akzepzieren (Signatur lautet dann void add(ItemUPtr &&item) ). Dann muss man zwar beim Aufrufen der Funktion auch ein move schreiben, aber dann ist klar was die Funktion macht.

MrSpoocy schrieb:

Vorweg zu der Frage warum ich bei den get Funktionen den Pointer und nicht ItemUPtr& zurück gebe... ich muss prüfen können ob wirklich ein Element zurückgegeben wurde. Auch wenn eben kein passendes Element gefunden wurde.

Es gibt natürlich auch leere unique_ptr . Der Grund ist wohl eher, dass du dann den unique_ptr aus der Map entfernen müsstest, weil diese wie der Name schon sagt einzigartig sind.

sebi707 schrieb:

MrSpoocy schrieb:

Vorweg zu der Frage warum ich bei den get Funktionen den Pointer und nicht ItemUPtr& zurück gebe... ich muss prüfen können ob wirklich ein Element zurückgegeben wurde. Auch wenn eben kein passendes Element gefunden wurde.

Es gibt natürlich auch leere unique_ptr . Der Grund ist wohl eher, dass du dann den unique_ptr aus der Map entfernen müsstest, weil diese wie der Name schon sagt einzigartig sind.

Ach ItemUPtr &. Da hast du recht.

MrSpoocy

Das mit dem add ist irgendwie auch nur eine not Lösung. Ich stand vor folgendem Problem:

Ich habe die ObjectList und irgendwo ganz wo anders in einer ganz anderen Klasse habe ich früher die Item's so erzeugt:

for ( irgendetwas )
{
Item item(id);
item.setXY();
item.setABC();

ObjectListPointr->add(item);
}

Das musste ich jetzt umstellen in

for ( irgendetwas )
{
auto item = std::make_unique<Item>(id);
item->setXY();
item->setABC();

ObjectListPointr->add(item);
}

damit die ObjectList der wirkliche Besitzer der Item's ist.

Was aus dem zweiten Beispiel allerdings nicht direkt ersichtlich ist, dass item nach dem ObjectListPointr->add(item) ungültig geworden ist. Bei dir kommt jetzt kein Code mehr aber vielleicht kommt man später mal auf die Idee noch irgendwas nach dem add mit item zu machen und dann gibts Probleme. Eindeutiger ist:

for ( irgendetwas )
{
  auto item = std::make_unique<Item>(id);
  item->setXY();
  item->setABC();

  ObjectListPointr->add(std::move(item));
}

Hier zeigt das einem das std::move direkt an, dass item ungültig geworden ist. Um das move zu erzwingen, muss deine add Funktion den Pointer als RValue Referenz nehmen. Im übrigen kann man bei deiner aktuellen Variante folgenden Code nicht schreiben:

ObjectListPointr->add(std::make_unique<Item>(id));

// Oder eventuell sinnvoller, geht aber auch nicht
std::unique_ptr<Item> makeItem(int id)
{
  auto item = std::make_unique<Item>(id);
  item->setXY();
  item->setABC();
  return item;
}

ObjectListPointr->add(makeItem(id));

Wenn man den unique Pointer als RValue Referenz nimmt geht das alles.

PS: Wenn der gezeigte Code doch mit deiner alten Funktion compiliert liegt das an einer nicht standardkonformen Erweiterung von Visual Studio. Ich hoffe echt, dass MS das endlich entfernt.