std::map und erase

theliquidwave

Hi.
Laut dem Artikel im C++ Magazin über die STL (Teil 1) hat std::map eine automatische Speicherfreigabe. Heißt dass, das folgender Code so funktioniert und keinen Memoryleak erzeugt?

class CTest
{
public:
    void append(char *pszName);
    void remove(char *pszName);

private:
    std::map<char*, CBla*, CmpStr> m_mapTest;
};

void CTest::append(char *pszName)
{
    char *pszKey = new char[strlen(pszName) + 1];
    strcpy(pszKey, pszName);
    CBla *pBla = new CBla;

    this->m_mapTest.insert(std::pair<char*, CBla*>(pszKey, pBla));
}

void CTest::remove(char *pszName)
{
    std::map<char*, CBla*, CmpStr>::iterator it = this->m_mapTest.find(pszName);

    if (it != this->m_mapTest.end())
    {
        this->m_mapTest.erase(it);
        // stimmt es nun, dass pBla und pszKey ab hier gelöscht sind?
    }
}

Eine zweite Frage: Wenn ich in einer Schleife bin, einen Eintrag lösche, ist der Iterator dann ungültig? Oder kann ich ihn trotzdem noch weiter verwenden zum inkrementieren? In etwa so:

void CTest::loop(void)
{
    for (std::map<char*, CBla*, CmpStr>::iterator it = this->m_mapTest.begin(); it != this->m_mapTest.end(); it++)
    {
        if (!strcmp(it->first, "bla"))
        {
            this->m_mapTest.erase(it);
            // hier nach schlägt it++ doch fehl, oder?
        }
    }
}

Gruß

Nexus

theliquidwave schrieb:

Laut dem Artikel im C++ Magazin über die STL (Teil 1) hat std::map eine automatische Speicherfreigabe. Heißt dass, das folgender Code so funktioniert und keinen Memoryleak erzeugt?

Nein. Die Speicherfreigabe betrifft die Objekte selber (in deinem Fall die Zeiger), und nicht etwaige referenzierte Speicherbereiche. Oft speichert man ja auch nicht-besitzende Zeiger.

Nimm doch einfach std::string , dann bist du das Gefrickel los.

theliquidwave schrieb:

Eine zweite Frage: Wenn ich in einer Schleife bin, einen Eintrag lösche, ist der Iterator dann ungültig? Oder kann ich ihn trotzdem noch weiter verwenden zum inkrementieren?

Wenn ein Element gelöscht wird, sind alle Iteratoren, Zeiger und Referenzen auf dieses Element immer ungültig. Du kannst über assoziative STL-Container wie folgt iterieren:

for (bla_map::iterator it = map.begin(), end = map.end(); it != end; /* nichts */)
{
    if (/* Element hat es nicht verdient zu leben */)
        map.erase(it++)
    else
        ++it;
}

hustbaer

Nexus schrieb:

Du kannst über sequenzielle STL-Container wie folgt iterieren:

for (bla_map::iterator it = map.begin(), end = map.end(); it != end; /* nichts */)
{
    if (/* Element hat es nicht verdient zu leben */)
        map.erase(it++)
    else
        ++it;
}

std::map ist kein sequenzieller Container, sondern ein assoziativer.

Und: man kann gerade über einen (sehr wichtigen) sequenziellen Container *nicht* so drüberiterieren, nämlich über std::vector.

Nexus

hustbaer schrieb:

std::map ist kein sequenzieller Container, sondern ein assoziativer.

Klar, ich meinte natürlich assoziativ. Danke für die Anmerkung, habe es korrigiert.

theliquidwave

Hi.
Danke!

Das heißt, ich muss pBla und pszKey (bzw. wenn ich dann std::string benutze nur pBla) selber löschen, richtig?

Zum iterieren: Folgendes Beispiel ist korrekt?

void CTest::loop(void)
{
    for (std::map<char*, CBla*, CmpStr>::iterator it = this->m_mapTest.begin(); it != this->m_mapTest.end(); )
    {
        std::cout << it->first;

        if (!strcmp(it->first, "test3"))
        {
            delete[] it->first;
            delete it->second;

            this->m_mapTest.erase(it);

            std::cout << " (deleted)";
        }
        else
        {
            it++;
        }

        std::cout << std::endl;
    }
}

void CTest::insert(void)
{
    this->m_mapTest.insert(std::pair<char*, CBla*, CmpStr>("test1", new CBla));
    this->m_mapTest.insert(std::pair<char*, CBla*, CmpStr>("test2", new CBla));
    this->m_mapTest.insert(std::pair<char*, CBla*, CmpStr>("test3", new CBla));
    this->m_mapTest.insert(std::pair<char*, CBla*, CmpStr>("test4", new CBla));
}

Ausgabe wäre dann also:

test1
test2
test3 (deleted)
test4

Und alles ist gelöscht (kein Memoryleak).

Edit @ Nexus: Kann es sein, dass dein Beispiel dann fehlschlagen würde? Ich dachte das Ende der Map verändert sich, wenn ein neues Item eingefügt oder ein anderes gelöscht wird, bei deinem wird aber immer auf das gleiche Element verwiesen (beim Vergleich in der for-Schleife).

Gruß & sorry wegen der vielen Fragerei

Nexus

theliquidwave schrieb:

Zum iterieren: Folgendes Beispiel ist korrekt?

Nein, du greifst auf den ungültigen Iterator zu. Wenn ein Element gelöscht wird, erhöhst du den Iterator nicht, also zeigt it immer noch auf das gleiche Element.

Aber noch was Allgemeines: Ich persönlich bin immer sehr vorsichtig, wenn es um manuelle Speicherverwaltung, C-Strings und ähnlichen Low-Level-Angelegenheiten geht. Ich kapsle solche Dinge meistens, um mit ihnen normal programmieren zu können. Normal heisst Wertsemantik und RAII, man muss nicht explizit freigeben oder sowas. Zwei delete s vor einem erase() sind schon etwas heikel, vor allem wenn man einige Zeit später wieder etwas erweitern möchte.

Zum Beispiel ist dein Code auch nicht exceptionsicher.

char *pszKey = new char[strlen(pszName) + 1];
    strcpy(pszKey, pszName);
    CBla *pBla = new CBla;

Wenn hier der Konstruktor von CBla oder der new -Operator eine Exception wirft, hast du ein irreversibles Memory Leak. Du sagst jetzt vielleicht, der Konstruktor wirft nichts. Aber mit grösster Wahrscheinlichkeit wirst du eben nicht an diesen Fall denken, wenn du in einem Monat die Klasse CBla plötzlich umgestaltest.

Im Weiteren kannst du deine std::map so nicht kopieren, ohne undefiniertes Verhalten zu riskieren. STL-Algorithmen sind auch gefährlich, weil sie von Wertsemantik ausgehen und Sequenzen umordnen können (ist bei assoziativen Containern nicht sehr relevant, aber allgemein). Zusammengefasst kannst du dir einige Probleme einhandeln, an die du gar nicht denkst. Wenn du vielleicht ein paar Mal einen Nachmittag lange wegen ungültiger Zeiger oder Memory Leaks debuggst, lernst du moderne C++-Konzepte wie RAII zu schätzen.

Gibt es überhaupt einen Grund, wieso du dynamische Speicherverwaltung brauchst? Ist CBla polymorph?

theliquidwave schrieb:

Edit @ Nexus: Kann es sein, dass dein Beispiel dann fehlschlagen würde? Ich dachte das Ende der Map verändert sich, wenn ein neues Item eingefügt oder ein anderes gelöscht wird, bei deinem wird aber immer auf das gleiche Element verwiesen (beim Vergleich in der for-Schleife).

Hm. Bei std::vector dürfte man das sicher nicht machen, aus dem erwähnten Grund. Ich bin mir nicht sicher, was der Standard darüber sagt, aber da andere Iteratoren in assoziativen Containern gültig bleiben, könnte das auf end() ebenfalls zutreffen.

Allerdings würde ich es nicht drauf ankommen lassen. Eigentlich ist das it = map.begin(), end = map.end; it != end lediglich eine blöde Angewohnheit von mir für reine Iterationsschleifen. Und auch da ist der Performancegewinn fragwürdig. Vielleicht sollte ich mir das wieder abgewöhnen, würde auch den Code um einiges schöner machen.