Element aus Map löschen

pumuckl

es gibt eine Funktion, die genau das tut, was du mit deiner Schleife zu erreichen versuchst. map::erase hat nämlich eine Überladung mit dem key-type:

myMap.erase(param);

pumuckl schrieb:

es gibt eine Funktion, die genau das tut, was du mit deiner Schleife zu erreichen versuchst. map::erase hat nämlich eine Überladung mit dem key-type:
myMap.erase(param);

Ich lösche aber nach dem Value und nicht nach dem Key^^

Skym0sh0

hab im internet irgendwo gefunden, dass man in solchen fällend en iterator nach dem löschen inkrementieren soll
dann ist der wieder gültig

ipsec

Skym0sh0 schrieb:

hab im internet irgendwo gefunden, dass man in solchen fällend en iterator nach dem löschen inkrementieren soll
dann ist der wieder gültig

Nein, einen ungültigen Iterator kann man nicht inkrementieren. erase gibt einen Iterator zurück, der hinter das gelöschte Element zeigt, mit diesem kann man dann weiter verfahren.

Also kann man mit einem ungültigen Iterator GARNIX mehr machen, oder?

Skym0sh0

also drauf zugreifen und dereferenzieren kann man nicht, wie denn auch wenn der ins nichts zeigt...
aber inkrementieren und dekrementieren müsste er können

CStoll

Skym0sh0 schrieb:

aber inkrementieren und dekrementieren müsste er können

Das dürfte genauso in die Hose gehen wie selber dereferenzieren - die meisten Iteratoren basieren auf Zeigern in die interne Container-Repräsentation und wenn das Element, auf das sie mal gezeigt haben, nicht mehr existiert, dann sind idR auch die Verwaltungsdaten weg an denen du dich durchhangeln willst.
Das einzige, was du mit einem ungültigen Iterator noch machen kannst, ist ihn wegzuschmeißen.

EOutOfResources

Man kann ihm einen Anderen zuweisen:

BrokenIter = ValidIter;

Skym0sh0

EOutOfResources schrieb:

Man kann ihm einen Anderen zuweisen:
BrokenIter = ValidIter;

xD

nein aber man könnte den iterator kopieren und dann vor dem erasen noch de- oder inkrementieren

CStoll

Ja, das wäre möglich, solange du es vor dem Löschen machst (und solange du mit einem knotenbasierten Container arbeitest - auf einem vector<> oder deque<> geht das Vorhaben vermutlich in die Hose).

Nexus

map.erase(it++);

Aber darauf achten, dass man den Iterator nicht noch zusätzlich inkrementiert.

Nexus schrieb:

map.erase(it++);
Aber darauf achten, dass man den Iterator nicht noch zusätzlich inkrementiert.

Uahh, da hat jemand seinen Zauberstab ausgepackt. Sowas macht Code finde ich leicht unlesbar

314159265358979

Gar nicht. Du hast ne Code-lese-Schwäche.

Nexus

TravisG schrieb:

Uahh, da hat jemand seinen Zauberstab ausgepackt. Sowas macht Code finde ich leicht unlesbar

Du kannst auch gerne das lesbarere

std::map<K, V>::iterator tmp = it;
++tmp;
map.erase(it);
it = tmp;

verwenden

Ich kann den Code klar lesen, es sieht fuer mich nur merkwuerdig aus. Nenne es Eitelkeit

Nexus schrieb:

TravisG schrieb:

Uahh, da hat jemand seinen Zauberstab ausgepackt. Sowas macht Code finde ich leicht unlesbar

Du kannst auch gerne das lesbarere
std::map<K, V>::iterator tmp = it;
++tmp;
map.erase(it);
it = tmp;
verwenden

Nein, ich wuerde schon deine Art bevorzugen. Waere eins dieser coolen Zauberstab-Dinge, die den Code schneller & kuerzer machen, und zusaetzlich den ein oder anderen verwirren koennten, wo man dann ein klein wenig Stolz ist (auch wenn mans nicht zugeben will), wenn man's einer Person erklaeren muss.

pumuckl

TravisG schrieb:

und zusaetzlich den ein oder anderen verwirren koennten, wo man dann ein klein wenig Stolz ist (auch wenn mans nicht zugeben will), wenn man's einer Person erklaeren muss.

Wenn man Stolz ist, weil man jemandem seinen Code erklären muss, hat man Mist gebaut. Code sollte zuallererst so aussehen, dass jeder, der wahrscheinlich damit in Verbindung kommt, ihn auch lesen kann. Wenn dein Code von Anfängern verstanden werden muss, dann gestalte ihn auch so, dass ein Anfänger ihn versteht - entweder durch klare Struktur oder durch genügend Kommentare. Kurzer, unlesbarer Code ist nicht cool sondern dumm und eine Fehlerquelle, wenn er gewartet werden soll.

EOutOfResources schrieb:

Man kann ihm einen Anderen zuweisen:
BrokenIter = ValidIter;

Kann man, nur braucht man dazu einen gültigen Iterator.

Skym0sh0 schrieb:

nein aber man könnte den iterator kopieren und dann vor dem erasen noch de- oder inkrementieren

CStoll schrieb:

Ja, das wäre möglich, solange du es vor dem Löschen machst (und solange du mit einem knotenbasierten Container arbeitest - auf einem vector<> oder deque<> geht das Vorhaben vermutlich in die Hose).

Dekrementieren geht auch bei vector. erase invalidiert dort nur Iteratoren nach dem gelöschten Element.

Nexus schrieb:

map.erase(it++);
Aber darauf achten, dass man den Iterator nicht noch zusätzlich inkrementiert.

Deshalb im Normalfall lieber dekrementieren. Der Inkrement für den nächsten Schleifendurchlauf setzt den Iterator dann wieder genau richtig. Da hier aber direkt danach break aufgerufen wird, ist it++ richtig.

Das assert am Ende ist aber dennoch falsch. Wenn das zu entfernende Element das letzte war, steht it nach dem Inkrement auf end() und das assert schlägt fälschlicherweise an. Das kann man auch nicht fixen, denn wenn die Map nur das eine zu löschende Element enthält, ist der end-Iterator hinterher der einzige mögliche iterator in die Map (weil identisch mit dem begin-Iterator). Ein assert, das sich nur auf Iteratoren stützt, kann hier also nicht zuverlässig funktionieren. Du müsstest also entweder vorher und hinterher die size() vergleichen, oder einen Algorithmus benutzen:

#include <algorithm>

/* ... */

  FooMap::iterator it = std::find_if(mFoos.begin(), mFoos.end(),
        //C++0x-Lambda als Praedikat, alternativ functor benutzen
        [&](FooMap::::value_type const& mapValue)
        {
          return mapValue.second == param;
        }
  );

  assert( (it != mFoos.end())  );  //Vergleichen, solange it noch gueltig ist

  mFoos.erase(it);

Alternativ gäbe es für deinen Code noch eine etwas unschöne Krücke mit einer boolschen Variablen:

bool gefunden = false;
    Foo::iterator it       = mFoos.begin();
    FooMap::iterator itEnd = mFoos.end();
    for(; it != itEnd; ++it) {
        if(it->second == param) {
            mFoos.erase(it);
            gefunden = true;
            break;
        }
    }

    assert( gefunden );

Ich benutze jetzt die Variante mit dem bool. Finde ich garnicht unschoen, sondern maximal einfach.

Das haessliche Konstrukt mit [&] sagt mir garnichts und ich verwende sowieso kein C++0x.

pumuckl

OP schrieb:

Ich benutze jetzt die Variante mit dem bool. Finde ich garnicht unschoen, sondern maximal einfach.

Ist vermutlich Geschmakssache, aber es ist ne Wegwerf-Variable. Ziemlich hässlich, finde ich.

Das haessliche Konstrukt mit [&] sagt mir garnichts und ich verwende sowieso kein C++0x.

Das hässliche Konstrukt nennt sich Lambda-Ausdruck und wird dir künftig immer häufiger unterkommen. MSVC 2010 kanns schon, gcc 4.5.x auch. Ist halt künftig Standard und bietet einige Vorteile

pumuckl schrieb:

Ist halt künftig Standard und bietet einige Vorteile

Du meinst die eh schon komplizierte C++ Syntax noch komplizieter machen? Ja, das haben sie geschafft;)

Nexus

pumuckl schrieb:

Deshalb im Normalfall lieber dekrementieren. Der Inkrement für den nächsten Schleifendurchlauf setzt den Iterator dann wieder genau richtig. Da hier aber direkt danach break aufgerufen wird, ist it++ richtig.

Dekrementieren finde ich nicht gut. Einerseits kanns beim begin() -Iterator ganz schön krachen, andererseits ist nichts tun besser als dekrementieren und wieder inkrementieren. Der Code ist klarer, wenn man einen else -Block und somit schön alle Fälle untereinander hat, als wenn man immer an das automatische ++it im Schleifenkopf denken muss.

Und drittens kann man nicht immer dekrementieren, was vor allem bei std::forward_list aus C++0x relevant werden dürfte.