Speicher freigeben: Aufrufer oder Klasse?

.filmor

So wie der Code jetzt ist ist er auf jeden Fall böse. Woher sollen die As wissen, dass ihr B* nicht mehr gültig ist? Du müsstest hier auf jeden Fall irgendwas mit Referenzzählung machen, am Besten automatisiert wie in Boost.SmartPointer.

Konrad Rudolph schrieb:

lolz schrieb:

wie soll a das objekt b löschen wenn mehrere As sich ein B teilen?

Das ginge schon, z.B. mit einem Referentzzähler in A. Aber ich sähe hier den Sinn nicht.

dann gibt a aber eine resource wieder frei und löscht sie nicht (ok, wenn die klasse nicht wohlgeformt ist, dann geht da sschon)

.filmor schrieb:

Du müsstest hier auf jeden Fall irgendwas mit Referenzzählung machen, am Besten automatisiert wie in
Boost.SmartPointer.

Ich bin mal grob drübergeflogen.

Bisher kannte ich nur auto_ptr.

Wäre shared_ptr hier das richtige für die B-Objekte?

.filmor

Ja. Zumindest mit den Informationen, die du gegeben hast.

.filmor schrieb:

Zumindest mit den Informationen, die du gegeben hast.

In meinem "richtigen" Programm gibt es "nur" den Unterschied, dass die Objekte der Klasse A selbst als Zeiger im Vector gespeichert werden (da es recht große Objekte sind):

class X
{
    private:
        std::vector<A*> viele_APointer;

}

Diese Zeiger werden im Destruktor der Klasse X gelöscht:

X::~X()
{
    for (int i = 0; i < viele_APointer.size(); ++i)
        delete viele_APointer[i];
}

Ändert das was an der Situation?

.filmor

Hmm, ich blick da im Moment nicht so richtig durch (kann auch an der Frühe liegen ;)).
So wie es jetzt aussieht ist doch die Zuständigkeit klar geregelt, die liegt nämlich bei dem X-Objekt?!

Simon2

.filmor schrieb:

Hmm, ich blick da im Moment nicht so richtig durch (kann auch an der Frühe liegen ;)).
So wie es jetzt aussieht ist doch die Zuständigkeit klar geregelt, die liegt nämlich bei dem X-Objekt?!

Das ist noch nicht zu sehen. Die Frage ist: Wer macht das "new" ?
Der Aufrufer ?

void X::add(A* a) { viele_A.push_back(a); }
int main(int argc, char* argv[]) {
    A* a = new A();
    X x;
    x.add(a);
    delete a; // BÖÖÖÖÖSE
};

... oder das X

void X::add(A& a) { 
    A* myA = new A(a); // kostet eine CopyKonstruktion
    viele_A.push_back(myA);
}

int main(int argc, char* argv[]) {
    A* pa = new A();
    A a;
    X x;
    x.add(*pa);
    delete pa; // macht nix
    x.add(a);
};

Alternativ kann man natürlich die Zuständigkeit komplett aus X herausverlegen:

class X
{
    public:
        X::~X() {}
        A* getA(int i) { return viele_APointer.at(i); }
        int getAcount() { return viele_APointer.size(); }
};
// Benutzer muss eine "delete-Schleife" implementieren

Aber das sieht nach einem selbstgestrickten "Container-Wrapper" aus, bei dem ich sehr vorsichtig wäre; da steckt der Teufel oft im Detail.

Gruß,

Simon2.

Weil das stückchenweise Code posten für etwas Verwirrung gesorgt hat, hier noch mal alles auf einen Blick, wie der aktuelle Stand der Dinge ist:

class B
{
//unwichtig (enthält keine Zeiger)
}

class A
{
    private:
        B* _b;
        int _y;
    public:
        A(B* b, int y) :_b(b), :_y(y) {}
}

class X
{
    private:
        std::vector<A*> viele_A;
    public:
        void add_A(A* a)
        {
            viele_A.push_back(a);
        }
        ~X()
        {
            for (int i = 0; i < viele_A.size(); ++i)
            {
                delete[i];
            }
        }
};

int main()
{
    B* b = new B;
    X x;
    x.add_A(new A(b, 0);
    x.add_A(new A(b, 1);
    x.add_A(new A(b, 2);
    delete b;
}

Also für mich stellt sich das wie folgt dar:

Es wird _einmal_ ein neues B auf dem Heap erzeugt.
In den Vector viele_A werden Speicheradressen auf Objekte der Klasse A auf dem Heap gespeichert.
-> diese Objekte werden auch schön wieder im Destruktor der Klasse X freigegeben.
Jedem Objekt der Klasse A wird als Parameter neben dem int-Wert noch die Adresse auf das Objekt der Klasse B auf dem Heap mitgegeben.
-> um die Freigabe dieses Objekts der Klasse B kümmert sich der Aufrufer mit delete b;

So, und jetzt seid ihr dran mir zu erzählen, warum dieser Code böse ist / wie man es anders machen sollte / ob smart pointer wie der shared_ptr nötigt sind.

.filmor

Du musst dir nur über die Zuständigkeiten Gedanken machen:
A -> wird von X verwaltet. (Falls A polymorph ist solltest du dir Boost.PointerContainer anschauen)
X -> liegt auf dem Stack
B -> nicht ganz eindeutig. Falls du SharedPointer verwendest, musst du den echten Zeiger direkt nach dem Einfügen unschädlich machen:

int main()
{
    X x;
    {
        B* b = new B;
        x.add_A(new A(b, 0);
        x.add_A(new A(b, 1);
        x.add_A(new A(b, 2);
    }
}

Das Beispiel ist aber meiner Meinung nach zu abstrakt, um eine Entscheidung für oder gegen SharedPointer zu fällen.

Simon2

Hi sly,

ich würde

"die As" von X erzeugen lassen und
"den As" nur ein B& übergeben.

Dann ist die Zuständigkeit klar.

Wenn Dein B tatsächlich ein "singleton" ist, könntest Du es in ein solches Pattern einsetzen .... wenn das aber aufwendiger als notwendig ist, kann man eine Referenz in den X-Ctor aufnehmen....

...und ganz sauber wäre es dann, wenn Du noch auf const-correctness achtest.
(Ich tue jetzt mal so, als ob A und X keine non-const-Methoden von B aufrufen)

class B
{
//unwichtig (enthält keine Zeiger)
}

class A
{
    private:
        const B& _b;
        int _y;
    public:
        A(const B& b, int y) :_b(b), :_y(y) {}
}

class X
{
    private:
        const B& _b;
        std::vector<A*> viele_A;
        // Kopieren unterbinden
        X(const X&);
        const X& operator=(const X&);
    public:
        X(const B& b) : _b(b) {}
        void add_A(int i) {
            viele_A.push_back(new A(b, i));
        }
        ~X() {
            for (int i = 0; i < viele_A.size(); ++i) {
                delete[i];
            }
        }
};

int main()
{
    B* b = new B;
    X x(*b);
    x.add_A(0);
    x.add_A(1);
    x.add_A(2);
    delete b;
}

Gruß,

Simon2.