[gelöst] factory pattern bei Klassen ohne default ctor

Dobi

Hallo zusammen,

gerade versuche ich mich an einer factory für klassen, die keinen Standard-Konstruktor haben.
Für das Beispiel hier habe ich RAII (SmartPtr), Nullpointerprüfungen und das Fangen der bad_cast-Exception weggelassen, damit es kürzer ist.

class Options {
public:
	virtual ~Options() = 0;
	virtual void DoSomething() {}
};

class Thing {
public:
	virtual ~Thing() = 0;
};

class AOptions : public Options {};

class AThing : public Thing {
public:
	AThing( const AOptions& ) {};
};

class BOptions : public Options {};

class BThing : public Thing {
public:
	BThing( const BOptions& ) {};
};

#include <string>

Options* AllocateOptions( std::string name ) {
	if ( name == "A" )
		return new AOptions;
	else if ( name == "B" )
		return new BOptions;
	else
		return 0;
}

Thing* AllocateThing( std::string name, const Options& options ) {
	if ( name == "A" ) {
		return new AThing( dynamic_cast<const AOptions&>( options ) );
	}
	else if ( name == "B" ) {
		return new BThing( dynamic_cast<const BOptions&>( options ) );
	}
	else
		return 0;
}

std::string GetTypeForFactory() {
	return "A";
}

int main() {
	std::string type = GetTypeForFactory();
	Options* optionsPtr = AllocateOptions( type );
	optionsPtr->DoSomething();
	Thing* thingPtr = AllocateThing( type, *optionsPtr );
}

Meine Variante von AllocateThing gefällt mir wegen des dynamic_casts allerdings überhaupt nicht. Hat jemand von euch eine bessere Idee? Vielleicht irgendwas cooles mit TMP?

Gruß
Dobi

Eisflamme

Also was ich hier nicht gut gelöst finde, ist, dass man theoretisch AllocateThing austricksen kann, indem man einen Namen übergibt, der nicht zu den Optionen passt.

Können Options auch anders als durch die Factory erzeugt werden, sodass man AllocateThing etwas anderes übergeben könnte? Falls nicht, was spricht dann gegen:

Thing* AllocateThing( std::string name) {
    Options* options = AllocateOptions(name);
    if ( name == "A" ) {
        return new AThing( options );
    }
    else if ( name == "B" ) {
        return new BThing( options );
    }
    else
        return 0;
}

Dobi

Ja, die Austricksmöglichkeit ist blöd. In dem Fall würde halt eine Exception fliegen. Wenn man die Options als Schlüssel nehmen würde, wär die Möglichkeit weg. Allerdings müsste ich ja dann dynamic_casts in alle Options-Varianten in AllocateThing durchprobieren, was ja zu ganz fiesen Problemen führen kann wenn man ne tiefere Hierarchie hätte.

Gegen das Allokieren der Options in AllocateThing spricht, dass mit den Options ja noch irgendwas gemacht werden soll (z.B. aus einer Datei einlesen) bevor das Thing konstruiert wird. Und das würd ich nur sehr ungern in die Factory-Funktion packen müssen. Vermeiden könnte man das durch ein Callback, aber das fänd ich noch ekeliger als das, was ich bisher habe.

314159265358979

Mir gefällt das irgendwie im ganzen nicht, dieses ganze if-else. Vielleicht wäre eine clone-Factory besser?
Dann würde ich eine map<name, prototype> verwenden und dann dementsprechend clone aufrufen.

Eisflamme

Dann muss ich aber deine ganze Options-Factory hinterfragen. Das Schöne ist doch gerade, dass die Options unterschiedlich erstellt werden können. Wenn die auch aus einer Datei geladen werden sollen, sollte das eben über Parameter oder Flags in den Options festgelegt werden können. In jedem Fall halte ich ein Erstellen über Factory und dann noch ein Nachladen nicht für sinnvoll, dann ist die Factory doch am Ende irgendwie nicht so toll.

314159265358979

Ich dachte da an etwa sowas:

struct clone_options
{
	clone_options() {}
	virtual ~clone_options(){}
};

struct clone_a_options : clone_options
{
	int new_value;

	clone_a_options(int new_value) : new_value(new_value) {}
};

struct clone_b_options : clone_options
{
	double new_value;

	clone_b_options(double new_value) : new_value(new_value) {}
};

struct Base
{
	virtual Base* clone(const clone_options& options) = 0; 
};

struct A
{
	int value;

	A(int value) : value(value) {}

	virtual Base* clone(const clone_options& options)
	{
		return new A(dynamic_cast<const clone_a_options&>(options).new_value);
	}
};

struct B
{
	double value;

	B(double value) : value(value) {}

	virtual Base* clone(const clone_options& options)
	{
		return new B(dynamic_cast<const clone_b_options&>(options).new_value));
	}
};

struct factory
{
	std::map<std::string, Base*> prototypes;

	Base* create(const std::string& name, const clone_options& options) const
	{
		auto iter = prototypes.find(name);

		if(iter == prototypes.end())
			return 0;

		return iter->second->clone(options);
	}
};

Dobi

@314159265358979: Nette Idee. Als prototype pattern gings natürlich auch. Die If-Kaskade wär dann weg, allerdings muss ich dann in jeder Klasse das clone implementieren und mein eigentliches Stil-Problem (dynamic_cast) ist dann immernoch da.

@Eisflamme: Die Options sehen in meinem Fall leider so aus, dass nur der richtige Options-Typ die Datei lesen kann, und der Typ auch nicht in der Datei (im Header oder sonstwo) steht.
Sie müssen übrigens auch nicht unbedingt aus einer Datei geladen werden. Wenn sie einfach nur erzeugt werden, haben die einzelnen Optionen in den Options schon Standardwerte.

Eisflamme

Ja, aber schau Mal: Factory soll Objekte erzeugen, sodass sie verwendet werden können. Aus Dateien laden gehört zum Initialisierungsprozess -> es gehört in die Factory.

Ansonsten hast Du die Verantwortlichkeit für das Vorbereiten des Objektes gesplittet, das empfinde ich als ziemlich hässlich und die Factory wird dann von mir hinterfragt bis sonst wo.

Das Laden könntest Du z.B. auch durch ein Flag an die Factory mitteilen oder durch Angabe eines Dateinamens. Und wenn das aber nicht immer der Fall sein muss, sollte das Objekt imo nicht durch die Factory erzeugt werden... wieso nutzt Du hier überhaupt Factorys, wie sieht denn für Dich der Vorteil dahinter aus?

Dobi

Ich benutze hier Factories, weil ich erst zur Laufzeit erfahre, was für Optionstypen das sind, die da eingelesen werden sollen.

Wenn ich so drüber nachdenke, komm ich auch zu dem Schluss, dass du Recht hast. Das Laden der Config gehört zum Erzeugen und somit in die Factory. Ob geladen werden soll, werde ich dann wie vorgeschlagen über fileName.empty() herausfinden. Vielen Dank.