3. Gottklasse, na und?

Gottklasse, na und?

  • E

    Hi,

    Mein heutiges Problem beschäftigt sich mit einer Gottklasse bzw. noch weiteren Unschönheiten meiner Klasse.

    Wie immer bin ich in der 3D-Programmierung unterwegs. Hier gibt es eine Klasse Model. Nun ist diese einfach Mal ein Riesenbatzen von Teilen:
    - Vertices
    - Indices
    - Normalenvektoren
    - Farben
    - Texturkoordinaten
    - Animationen
    - Nodes (Hierarchie von Teilen des Models)
    - Bones (für Animationen)
    - Boundingboxen/Spheres für Kollisionsabfragen
    - viele weitere Eigenschaften werden künftig kommen für Bummaps/Normalmaps und weiß der Geier

    Problem 1
    Ich muss ehrlich gestehen, dass ich das direkte Problem der Gottklasse nicht sehe. Gut, das zählt als Antipattern, wir haben hier einen Riesenhaufen an Methoden und Attributen, typedefs natürlich auch wegen der Zahlreichen vector/map-Attribute welche irgendwelche Koordinaten aggregieren.

    Es gibt die Möglichkeit, dass ich ein paar der Attribute zusammenfasse und daraus wiederum ein Objekt baue. Ich muss aber dazu sagen, dass das sehr künstlich wäre. Außerdem vergrößert sich dadurch mein Zugriffspfad auf einzelne Elemente.
    Zur Zeit habe ich einige der Teile fast schon willkürlich in eine so genannte Mesh-Klasse ausgelagert, die einst nur für das statische Rendern des Objektes gedacht war. Bei einer Animation werden aber wiederum die anderen Objekte benötigt etc., sodass ich an das Mesh alles einmal durchreichen darf. Da habe ich außer einer scheinbaren Übersichtlichkeit nichts gewonnen.

    Problem 2
    Eine andere Sache, welche die Klasse aufbläht, und mich irgendwie stört:
    Das Model kann durch mehrere verschiedene Loader aufgebaut werden. Angedacht sind mindestens zwei Loader für verschiedene Formate. Jetzt braucht die Loaderklasse Zugriff auf die Modelklasse. Also habe ich in dieser einen Haufen Getter spendiert, welche nicht-konstante Referenzen zurückliefern auf alle maps und vectors. 😞 friend hat den Nachteil, das für jeden neuen Modelloader mein Model geändert werden muss...

    Hm... 💡 Was haltet ihr von folgender Idee: Alle ModelLoader erben von einer Basisklasse, Model ist mit dieser befreundet und die Basisklasse hat die entsprechenden Referenzgetter der Form:

    protected: Model::VertexMap& GetVertexMap() {return model_->vertexMap_;}

    Damit würden nur Loader Zugriff auf das Ding kriegen, gefällt mir irgendwie. 🙂

    Problem 3
    Falls die anderen beiden Punkte nicht schon genug Zeit in Anspruch nehmen, habe ich hier noch ein Randproblem, das man eigentlich auch in einen anderen Thread auslagern könnte (wieso findet man hier im Forum eigentlich so selten Designfragen, wie ich sie habe. Ich hab manchmal das Gefühl einfach ein Brett vor dem Kopf zu haben 🙄).

    Und zwar macht es keinen Sinn, dass man eine ModelLoader-Klasse (also ein Derivat, aber ich spreche jetzt Mal nur von ModelLoader-Klasse) mehrfach instanziiert. Es gibt auch kein Attribut bis auf das neu erstellte Model während des Ladeprozesses, da ich diverse Funktionen zum Beladen der Einzelteile habe.

    Also ist das eigentlich keine Klasse, oder? Wir sind ja hier nicht in Java. Singleton-Pattern geht, aber das nützt ja eigentlich auch nichts. Zur Zeit mache ich eine ModelLoader-Instanz, wenn ich sie brauche.

    Die Alternative wären freie Methoden in einem Namespace, aber ist das so toll? Vielleicht ist die Klassendefinition (eines Derivats) hilfreich:

    namespace Engine
{
	class DLL_SPECIFIER AssimpModelLoader : public ModelLoader
	{
	private:
		typedef const aiScene*	aiScenePtr;
// Assimp ist eine 3D-Bibliothek zum Laden;
// Diese Klasse dient damit als eine Art Wrapper, Middleware-class, welche
// die Assimp-Daten in die Struktur meiner Anwendung bringt
	private:
		// Assimp Importer
		static Assimp::Importer		importer_;
		// Temporary AI Scene
		aiScenePtr					temporaryScene_;
		unsigned int				numberOfVertices_;
		Model*						newModel_;

	private:
		void	getNumberOfVertices();
		void	extractMaterials				(const std::string& filename);
		void	integrateVertices				();
		void	integrateIndices				();
		void	integrateTextureCoordinates		();
		void	integrateNormals				();
		void	integrateColors					();
		void	integrateTangents				();
		void	integrateNodes					();
		void	integrateBones					();
		void	integrateBoneIndicesAndWeights	();
		void	integrateAnimations				();

		void	integrateSubNodes			(ModelNode& modelNode, const aiNode& aiNode);

		UsualVector		ConvertVector(const aiVector3D& vector);
		UsualMatrix		ConvertMatrix(const aiMatrix4x4& matrix);
		UsualQuaternion	ConvertQuaternion(const aiQuaternion& quaternion);

	public:
		virtual Model* LoadModel(const std::string& filename);

		AssimpModelLoader();
	};
}

    Und LoadModel ruft die privaten Methoden alle auf. Die Kapselung hätte ich mit globalen Methoden natürlich nicht, rechtfertigt das eine Klasse schon? Trotzdem fühle ich mich irgendwie nicht ganz wohl dabei.

    Sooo, langer Post, tut mir Leid. Ich hoffe, die Probleme sind klar geworden und ich freue mich wie immer auf kritische Antworten 😋

    Vielen Dank im Voraus!

    Viele Grüße!

    0
  • K

    1. Wenn alle Loader sowieso Referenzen auf die Interna der Klasse erhalten, dann machen getter/setter keinen Sinn.
    2. Freunde werden nicht vererbt.
    0
  • E

    Tut mir Leid, ich möchte nicht undankbar erscheinen, aber irgendwie kann ich darauf nicht viel mehr sagen, als dass ich nicht das Gefühl hab, als hättest du meinen Text richtig gelesen.

    Wenn etwas nicht unklar beschrieben ist, nehm ich die Kritik gern entgegen. Bei 2 meinte ich übrigens folgende Idee:

    class Y;

class X
{
private:
	int value;

	friend class Y;

public:
	X(int v) :value(v){}
};

class Y
{
protected:
	int GetX(X x) {return x.value;}
};

class Z : public Y
{
public:
	void test(X x)
	{
		std::cout << GetX(x);
	}
};

    Z muss nicht Freund von X sein, daher braucht die Freundschaft auch nicht vererbt zu werden.

    0
  • ?

    edit: hier stand käse, hätte problem 3 auch lesen sollen :p

    0

  • zu Problem 3:

    Wenn es keinen Grund gibt im Interface eine Klasse zu verwenden, dann mach eine freie Funktion draus.
    Das heisst aber nicht dass es in der Implementierung nicht eine Klasse geben kann. Stichwort Method-Object.
    D.h. du lässt deine Klasse im Prinzip so wie sie ist, und definierst eine Funktion ala

    Model* LoadModel(const std::string& filename)
{
    Detail::ModelLoader loader;
    return loader.LoadModel(filename);
}

    Die ModelLoader Klasse kann dabei entweder in einem Detail-Namespace definiert sein, oder in einem anonymen Namespace direkt im .cpp File, oder überhaupt gleich direkt lokal in der LoadModel-Funktion (ist mir persönlich bei grösseren Klassen aber zu unübersichtlich).

    D.h. im (public) Interface gibt es diese Klasse nicht. Ebensowenig sind die diversen Hilfsfunktionen extractMaterials, integrateVertices etc. öffentlich zugänglich.

    zu Problem 2:
    Naja... da gibt es viele Möglichkeiten, die alle Sinn machen können, je nachdem was man will/braucht.

    1: Du verpasst der Model Klasse die nötigen Mutator-Funktionen, und arbeitest mit const . D.h. überall wo das Model nur mehr rumgereicht werden soll, aber nicht modifiziert, arbeitest du nur mit Model const Zeigern/Referenzen.

    2: Du verpasst der Model Klasse einen oder mehrere Konstruktoren, an die du sämtliche benötigten Daten übergibst. Das Model ist als nach dem der Constructor fertig gelaufen ist vollständig erstellt, und lässt sich danach nicht mehr modifizieren. Sämtliche Methodon von Model sind const .

    3: Du baust eine ModelBuilder Klasse ala StringBuilder in Java. Die ModelBuilder kann dabei z.B. friend von Model sein, und jeder der ein Model erstellen will verwendet die Builder-Klasse.

    0
  • E

    Okay. Aber auf den Detail-Namespace könnte doch jetzt ohne weiteres auch jemand anderes zugreifen, oder? Ein anonymer namespace in einer cpp-File würde bedeuten, dass ich dort wie in Java direkt Klassendefinition und -implementierung zusammenschmeiße? Ich hab ganz gerne die Übersicht eine Stufe darüber...

    Die globale Zugriffsmethode gefällt mir intuitiv sehr gut. Aber letztlich sorgt das ja nur dafür, dass der Aufrufer keine zusätzliche Instanz erstellen muss. Wo liegt da denn genau der Vorteil?

    3 ist ja so ähnlich wie bei meiner Idee, nur dass ich in meiner Variante über Vererbung gelaufen wäre, was sich aber anbietet, da die Klasse ja sowieso besteht. Und über die protected Methoden der Basisklasse kommt die abgeleitete Klasse halt an die Referenzen. Hat das irgendwelche Nachteile?

    Der dicke ctor sorgt halt für das gleiche Problem, was ich bei 1 habe -> Gottklasse. Dafür noch irgend eine Idee? Oder ist es in Ordnung, das bei dieser Struktur (die nebenbei bemerkt auch performancekritisch ist), so zu belassen?

    0
  • T

    Du weisst ja schon, dass eine Gottklasse als Anti-Pattern gilt. Auf den Seiten, welche das sagen, steht nicht nur, dass es ein Antipattern ist, sondern auch warum es eines ist.

    0
  • E

    Ja, aber die gehen mir nicht genug in die Tiefe:
    - Wiederverwendbarkeit
    => inwiefern? Alle Unterobjekte, die ich erzeugen würde, würden trotzdem nur durch "Model" genutzt werden, die machen alleine wenig Sinn

    - wenn man die Klasse in Unterklassen gliedert, lässt sich das System besser ändern/erweitern
    => finde ich wieder schwierig... ich habe nun Mal ständig 1 : 1 von Model zu den Unterklassen, die ich dann hätte. Zählt das dann noch?

    Überhaupt stört mich etwas, dass ich hier eine große Klasse in Unterklassen auslagern würde, wobei halt immer alles 1:1 ist und auch 1:1 bleibt, ergibt das dann überhaupt Sinn?

    - Abhängigkeiten; alle nutzen Model, alle lieben Model... Model, Model, Model
    => Unterklassen wären sowieso nur innerhalb des Models instanziiert, also müsste man die Unterklassen auch über diese Klasse ansteuern

    Hab ich was vergessen? Ich finde gerade nicht wirklich besseres Material.

    Dann frage ich mich allerdings, ob meine Klasse überhaupt eine Gottklasse ist, nur weil sie viele Methoden und Attribute hat. Single Responsibility habe ich hier eigentlich dennoch...

    Darum sage ich ja, ich bin nicht so sicher, ob das ein Problem ist. Es sieht bisher nur "nicht schön aus", aber wenn ich am Ende merke, dass alles zusammenfällt, denke ich mir "hätte ich das Mal vorher gesehen 🙄".

    0
  • E

    Keiner ne Idee? 😞

    0
  • Mod

    Eisflamme schrieb:

    Keiner ne Idee? 😞

    Du redest immer nur vom jetzt. Warum bist du so sicher, dass du die Teile der Funktionalität niemals für etwas anderes brauchen wirst? Woher weißt du, dass du nie Erweiterungen/Änderungen an Teilen vornehmen wirst?

    Wenn du diese Fragen wirklich zufriedenstellend beantworten kannst, dann bleib dabei. Aber die Erfahrung der ganzen Welt steht dir entgegen, dass dies nicht der Fall sein wird.

    0
  • ?

    Eisflamme schrieb:

    Keiner ne Idee? 😞

    schau dir andere Loaderframeworks und Graphicengines an.

    0
Anmelden zum Antworten