Bin ich mit richtig hier löschen?

Question

meine Kenntnisse in C ++ kombinieren Klassen und dynamische Arrays

Ich habe gerade erst begonnen. Ich habe den Rat gegeben, dass „jedes Mal wenn ich den neuen Operator“ Ich löschen soll. Ich weiß auch, wie Destruktoren Arbeit, so dass ich denke, dieser Code korrekt ist:

main.cpp

...
int main()
{
    PicLib *lib = new PicLib;
    beginStorage(lib);
    return 0;
}

void beginStorage(PicLib *lib)
{
...
    if (command != 'q')
    {
        //let's assume I add a whole bunch
            //of stuff to PicLib and have some fun here
        beginStorage(lib);
    }
    else
    {
        delete lib;
        lib = NULL;
        cout << "Ciao" << endl;
    }
}

PicLib.cpp

...

PicLib::PicLib()
{
    database = new Pic[MAX_DATABASE];
    num_pics = 0;
}

PicLib::~PicLib()
{
    delete[] database;
    database = NULL;
    num_pics = 0;
}
...

Ich fülle meine PicLib mit einem Pic Klasse, dynamischen Arrays enthält. Pic Destruktor löscht sie auf die gleiche Weise oben gesehen. Ich denke, dass delete [] database loswerden alle richtig diesen Klassen wird.

ist also das Löschen in main.cpp notwendig? Alles sucht beste Ordnung hier?

Answer 1

Es gibt ein paar Probleme:

int main() 
{ 
  PicLib *lib = new PicLib; 
  beginStorage(lib); 
  return 0; 
}

Es ist am besten zu verteilen und zu löschen Speicher in dem gleichen Umfang, so dass es einfach ist, zu erkennen.

Aber in diesem Fall nur lokal erklären (und übergibt Bezug genommen wird):

int main() 
{ 
    PicLib  lib; 
    beginStorage(lib); 
    return 0; 
}

In beginStorage ()

Aber ich sehe keinen Grund, einen Zeiger zu manipulieren. Direkt durch Bezugnahme und es nur lokal verwendet werden.

void beginStorage(PicLib& lib)
{
 ....
}

In der PicLib Klasse haben Sie einen RAW-Zeiger: Datenbanken

.

Wenn Sie eine RAW-Zeiger haben, dass Sie besitzen (Sie erstellen und zerstören), dann müssen Sie die Compiler erzeugt Versionen des Copykonstruktor Zuweisungs-Operator überschreiben. Aber in diesem Fall sehe ich keinen Grund touse einen Zeiger wäre es einfacher, nur einen Vektor verwenden:

class PivLib
{
    private:
        std::vector<Pic>   databases;
};

Answer 2

Ja, alles, was Sie erstellen mit neue muss mit gelöscht werden Löschen , und alles, erstellt mit new [] mit delete [] .

Es gibt einige Dinge, die ich in Ihrem Code hinweisen würde aber:

• Bevorzugen std :: vector <> über den Einsatz neuer [] und delete []. Es wird für Sie die Speicherverwaltung tun. Haben Sie auch einen Blick auf intelligente Zeiger wie auto_ptr, shared_ptr und in C ++ 0x unique_ptr für automatische Speicherverwaltung. Diese helfen, speichern Sie einen Lösch zu vergessen und verursacht ein Speicherleck. Wenn Sie können, nicht neu verwenden / new [] / delete / delete [] auf allen!
• Wenn Sie neue und löschen etwas haben, es ist ein sehr gute Idee , um neu in eine Klasse Konstruktor, und löschen Sie in einer Klasse destructor. Wenn Ausnahmen geworfen werden, Gegenstände gehen aus Umfang usw., werden ihre Destruktoren automatisch aufgerufen, so dass diese Speicherlecks verhindern hilft. Es heißt RAII.
• Mit beginStorage löschen seine Parameter ist möglicherweise eine schlechte Idee. Es könnte abstürzen, wenn Sie die Funktion mit einem Zeiger nicht erstellt mit Aufruf neue , weil Sie einen Zeiger nicht löschen können. Es wäre besser, wenn main () ein PicLib auf dem Stapel erstellt anstatt neue und löschen, und für beginStorage eine Referenz zu nehmen und nicht löschen nichts.

Answer 3

Ja, es ist notwendig, es sei denn, Sie ein auto_ptr verwenden (und auf die Semantik von auto_ptr lesen, bevor Sie es benutzen - Sie können kopieren nicht arround).

Beispiel:

int main()
{
    auto_ptr<PicLib> lib = new PicLib;
    beginStorage(lib);
    return 0;
} // auto_ptr goes out of scope and cleans up for you

Answer 4

else geht nicht mit while. Sie würden wollen, etwas mehr wie:

void beginStorage(PicLib *lib)  
{  
    while (command != 'q') 
    { 
        //let's assume I add a whole bunch 
            //of stuff to PicLib and have some fun here 
    } 

    delete lib; 
    lib = NULL;  // Setting to NULL is not necessary in this case,
                 // you're changing a local variable that is about
                 // to go out of scope.
    cout << "Ciao" << endl;
}

Die delete sieht gut aus, aber Sie sollten sicher zu dokumentieren, dass beginStorage Besitz des Objekts PicLib nehmen. Dass jeder Weg mit beginStorage weiß, dass sie müssen es nicht später löschen.

Answer 5

Der Lösch in main.cpp notwendig ist.

Dies ist wahrscheinlich eine Frage des persönlichen Geschmacks, aber ich würde davon abraten neuen Aufruf und lösche in separaten logischen Teilen (hier der Lösch Anruf auf der PicLib Instanz in einer separaten Funktion ist). Normalerweise ist es besser, die Verantwortung für die Zuweisung und Freigabe gegeben nur einen Teil.

@AshleysBrain hat einen besseren Vorschlag (über das Erstellen von PicLib der Stapel), obwohl dies könnte zu Problemen führen, wenn PicLib nimmt zu viel Speicher.

Answer 6

Im Allgemeinen wollen Sie neu an der gleichen Stelle, wie Sie löschen. Es macht die Buchhaltung einfacher. Besser ist es, einen Smart-Pointer (scoped_ptr in diesem Fall) zu verwenden, was bedeutet, dass der Code immer noch richtig ist, auch wenn der Körper des während einer Ausnahme und beendet wirft vorzeitig.

Answer 7

Alles sieht gut aus.

Der Lösch in main.cpp ist notwendig, weil, wenn Sie nicht löschen angerufen haben dann der destructor nicht ausgeführt und das Array wird nicht gelöscht. Sie würden auch den Speicher für die Klasse undicht sein, nicht nur das Array.

Answer 8

Ja, sonst wird der Destruktor für PicLib nicht in Anspruch genommen werden.

Eine stilistische Note aber: Wenn Sie ein Methode-scope Objekt in einer Funktion neu, versucht, es in der gleichen Funktion zu löschen. Als Junior-Ingenieur, die anderen Leute Speicherlecks Fixierung durch große Projekte gehen mußten ... macht diese Dinge viel einfacher für andere Menschen zu lesen. Von den Blicken von ihm, können Sie * lib nach beginStorage () zurückkehrt löschen. Dann wäre es einfacher, den Umfang des * lib an einem Ort zu sehen.