Eine Art Factory

Hallo!

Es liegen folgende Klasse vor:

class FooBase
{};

class Foo1 : public FooBase
{};

class Foo2 : public FooBase
{};

Nun soll mit den Klassen innerhalb einer Methode (einer anderen Klasse) gearbeitet werden. Je nach Auswahl aber mit Foo1 oder Foo2.

class MyClass
{
public:
    void DoSomething(FooBase & fb);
};

Aufgerufen wird DoSomething bisher so:

void AnotherClass::Method(const int & Id)
{
    if (Id == 0)
        this->m_MyClass.DoSomething(Foo1());
    else
        this->m_MyClass.DoSomething(Foo2());
}

Diese if-Abfrage stört mich, denn sie ist zu umständlich, da später später noch mehr von FooBase abgeleitete Klassen hinzukommen werden. Ich dachte an so etwas wie eine Factory mit einem auto_ptr als Member, da in MyClass::DoSomething Ausnahmen geworfen werden.

Meine erdachte Lösung. Eine neue Factory Klasse, die die if-Abfragen kapselt.

class FooFactory
{
private:
    std::auto_ptr<FooBase *> m_pFooBase;
public:
    FooBase & Create(const int & Id)
    { 
        if (Id == 0)
            this->m_pFooBase = new Foo1;
        else
            this->m_pFooBase = new Fo21;

        return *m_pFooBase
    }
};

Ist das so möglich und wird m_pFooBase auch wirklich bei Ausnahmen gelöscht? Gibt es vielleicht eine andere, wesentlich bessere Herangehensweise?

Gruß Klaas

Nexus

Würde sich switch nicht eignen, oder sehe ich da etwas falsch? Musst du für die Erstellung der FooBase -Derivate eine eigene Klasse haben, reicht eine Funktion nicht?

Und ja, wenn du std::auto_ptr verwendest, wird der Speicher beim Verlassen des Scopes auf jeden Fall freigegeben, also auch bei Exceptions (vorausgesetzt, dein Smart Pointer besitzt das Objekt).

Nexus schrieb:

Würde sich switch nicht eignen, oder sehe ich da etwas falsch? Musst du für die Erstellung der FooBase -Derivate eine eigene Klasse haben, reicht eine Funktion nicht?

switch nimmt sogar mehr Platz ein als die if-Abfragen. Ich möchte meine Funktion nicht so unnötig aufblähen, daher wollte ich den Code in eine Factory auslagern.

Nexus schrieb:

Und ja, wenn du std::auto_ptr verwendest, wird der Speicher beim Verlassen des Scopes auf jeden Fall freigegeben, also auch bei Exceptions (vorausgesetzt, dein Smart Pointer besitzt das Objekt).

Was eminst Du mit "dein Smart Pointer besitzt das Objekt"? Er wird in FooFactory::Create nur dereferenziert und als Referenz zurückgegeben um in anderen eine Referenz anstatt eines Zeigers zu nutzen. Diese Referenz wird ganz "normal" genutzt.

Nexus

Klaas123 schrieb:

switch nimmt sogar mehr Platz ein als die if-Abfragen. Ich möchte meine Funktion nicht so unnötig aufblähen, daher wollte ich den Code in eine Factory auslagern.

Gut, aber bei vielen Fällen kann sich switch schnell einmal auszahlen. Platz braucht es auch nicht mehr als if , sogar eher weniger.

switch (ID)
{
    case 1:   return new Foo1();   break;
    case 2:   return new Foo2();   break;
    case 3:   return new Foo3();   break;
    default:  return new FooD();
}

Klaas123 schrieb:

Was eminst Du mit "dein Smart Pointer besitzt das Objekt"? Er wird in FooFactory::Create nur dereferenziert und als Referenz zurückgegeben um in anderen eine Referenz anstatt eines Zeigers zu nutzen. Diese Referenz wird ganz "normal" genutzt.

Meine Aussage ist eigentlich nicht so weltbewegend. Ich meinte nur Folgendes:

std::auto_ptr<foo> function()
{
    std::auto_ptr<foo> foo_ptr(new foo());

    if (...)
        throw std::runtime_error("bla"); // zerstört das Objekt beim Verlassen des Scopes.
    else
        return foo_ptr; // hier wird das Objekt natürlich nicht zerstört.
}

Also wenn der Smart-Pointer das Objekt nicht mehr besitzt (d.h. er zeigt auf Null, da er den Besitz weitergegeben hat), gibt er dieses logischerweise auch nicht frei. Tut mir leid, wenn ich deswegen Verwirrung gestiftet habe.

Nexus schrieb:

Gut, aber bei vielen Fällen kann sich switch schnell einmal auszahlen. Platz braucht es auch nicht mehr als if , sogar eher weniger.

So stimmt, das zwar, jedoch würde das gegen "meine" Codierrichtlinien verstoßen, da das return statement nicht in der gleichen Zeile stehen darf. Zudem ist das break doch sogar überflüssig, oder?

Nexus schrieb:

Also wenn der Smart-Pointer das Objekt nicht mehr besitzt (d.h. er zeigt auf Null, da er den Besitz weitergegeben hat), gibt er dieses logischerweise auch nicht frei. Tut mir leid, wenn ich deswegen Verwirrung gestiftet habe.

Jetzt bin ich erst recht verwirrt.
Also, in der FooFactory -Klasse ist doch der std::auto_ptr ein Member und ich gebe hier eine Referenz auf das Objekt zurück. Somit wird doch nichts hin und her kopiert und das Objekt bei einer Exception gelöscht. Dachte ich zumindest...

Gruß
Klaas

Nexus

Klaas123 schrieb:

So stimmt, das zwar, jedoch würde das gegen "meine" Codierrichtlinien verstoßen, da das return statement nicht in der gleichen Zeile stehen darf.

Okay. Es wäre halt auch schneller als mehrere if -Abfragen. Und wenn du schon eine eigene Klasse für die Erzeugung hast, dürfte ein bisschen mehr Code eigentlich nicht dramatisch sein (obwohl es eigentlich nicht wirklich mehr ist, auch wenn du die case s und return s auf verschiedene Zeilen schreibst).

Klaas123 schrieb:

Zudem ist das break doch sogar überflüssig, oder?

Argh, natürlich. Ich hatte es zuerst auch nicht drin, beim Überfliegen des Codes hab ich dann gedacht: "oh, ein switch ohne break" und ohne viel zu überlegen break hingeschrieben.

Klaas123 schrieb:

Jetzt bin ich erst recht verwirrt.
Also, in der FooFactory -Klasse ist doch der std::auto_ptr ein Member und ich gebe hier eine Referenz auf das Objekt zurück. Somit wird doch nichts hin und her kopiert und das Objekt bei einer Exception gelöscht. Dachte ich zumindest...

Das stimmt schon. Ich meinte eigentlich nur, dass der Smart-Pointer nicht mehr für die Freigabe verantwortlich ist, sobald er seinen Besitz an einen anderen std::auto_ptr weitergegeben hat. Das heisst nicht, dass du ein Memory Leak hast (das sicher nicht, der Speicher wird sicher freigegeben), aber die Freigabe erfolgt eben nicht unter allen Umständen beim Verlassen des Scopes.

Dafür ist mir noch was anderes aufgefallen:

std::auto_ptr<FooBase *> m_pFooBase;

müsste wohl so heissen:

std::auto_ptr<FooBase> m_pFooBase;

Ausserdem ist der std::auto_ptr<T>::operator= nicht für Zeiger überladen, statt

this->m_pFooBase = new Foo1;

sollte dort

this->m_pFooBase.reset(new Foo1);

stehen (auch wenn ichs ohne this-> machen würde).

Nexus schrieb:

Dafür ist mir noch was anderes aufgefallen:

Danke für die Hinweise, wird mir sicherlich ein paar Compiler-Fehler sparen, jedoch sollte es nur Demo-Code sein (wobei auch dieser sytaktisch korrekt sein sollte ).