Tipps zur Codestruktur

05/30/2013 09:49 Padmak#1
Moin epvp,

ich wurde durch den Singleton-Thread ([Only registered and activated users can see links. Click Here To Register...]) darauf aufmerksam gemacht, dass mein aktueller Code nicht sonderlich schön aufgeteilt ist. Außerdem ist es mit einem Wunsch zusammengefallen, meine Applikation wirklich Multiuserfähig zu machen...

Ich zeig euch einfach Code, das kann ich besser als Deutsch..
ACHTUNG, ALLES PSEUDOCODE ZUM VERDEUTLICHEN
So war es bisher:
Code:
class Inventory : public Singleton<Inventory>
{
public:
	void SetItemData(ItemData&);
	void GetItemData(ItemData&);
};

class Network : public Singleton<Network>
{
public:
	void MainLoop()
	{
		if( (recvd = recv(&buf)) > 0)
		{
			PacketHandler::Instance()->OnRecvPackets(&buf, recvd);
		}
};

class PacketHandler : public Singleton<PacketHandler>
{
public:
	void OnRecvPacket(uint8_t* data, size_t len)
	{
		buf_queue.insert(buf_queue.end(), data, data + len);
		MainPacketHandler();
	}
	void MainPacketHandler()
	{
		uint8_t packet_id = ((uint8_t*)&buf_queue[0])[0];
		switch(packet_id)
		{
			case ITEM_ADD:
				Inventory::Instance()->SetItemData(...);
				buf_queue.erase(buf_queue.begin(), sizeof(ItemAddPacket));
		};
	}
private:
	std::vector<uint8_t> buf_queue;
};
Dass das Schwachsinn ist, ist mir irgendwie jetzt auch klar...
Darum dachte ich, ich könnte eine einzige Klasse machen, die so aussieht:
Code:
class Main
{

private:
	Inventory m_inventory;
	Network m_network;
	PacketHandler m_handler;
};
Jetzt habe ich aber das Problem, dass die einzelnen Klassen nach wie vor aufeinander Zugreifen müssen.
Hier kommt meine Frage:
Soll ich lieber jeder einzelnen Klassen Referenzen auf die benötigten geben, so:
(Am Beispiel der Netzwerk-Klasse)
Code:
class Network : public Singleton<Network>
{
public:
	Network(Main* inst, PacketHandler* handler) 
	{ 
		m_client_instance = inst; 
		m_packet_handler = handler;
	}
	void MainLoop()
	{
		if( (recvd = recv(&buf)) > 0)
		{
			m_packet_handler->OnRecvPackets(&buf, recvd);
		}
private:
	Main* m_client_instance;
	PacketHandler* m_packet_handler;
};
Oder lieber in der Main-Klasse alle "Durchsatzfunktionen" neu definieren, also so:
Code:
class Network : public Singleton<Network>
{
public:
	Network(Main* inst) 
	{ 
		m_client_instance = inst; 
	}
	void MainLoop()
	{
		if( (recvd = recv(&buf)) > 0)
		{
			m_client_instance->OnRecvPackets(&buf, recvd);
		}
private:
	Main* m_client_instance;
};
class Main
{
public:
	void OnRecvPacket(uint8_t* data, size_t len)
	{
		m_handler.OnRecvPackets(data, len);
	}
private:
	Inventory m_inventory;
	Network m_network;
	PacketHandler m_handler;
};
Ooooder die Main-Klasse von allen anderen erben lassen (was ich aber für die schlechteste Alternative halte)

Oder aber ist alles Bullshit und die wahre Lösung ist ganz anders?

Wäre nett, wenn jemand das versteht und helfen kann :D

Fehler/Ungereimtheiten im Code könnten sein, ich hab das grade nur aus'm Kopf auf das wesentliche reduziert geschrieben.. Einfach nachfragen und/oder drauf aufmerksam machen.

epvp, do your thing!

€: Ich probier grade rum und muss sagen: das mit Vererbung wirkt gar nicht SOO doof. Meinungen?

Padmak
05/30/2013 11:54 Schlüsselbein#2
Ganz einfach: _Die_ Lösung gibt es hier nicht. Dir sind vielleicht ja die Entwurfsmuster MVC, MVP und Dependency Injection bekannt.

Erster Vorschlag von dir geht eher Richtung DI und zweiter in Richtung MVC/MVP, wobei deine Main-Klasse Controller/Presenter ist.
Beides sind völlig legitime Vorgehensweisen. Allgemein sollte man sich nicht zu sehr auf die Muster versteifen, sie dienen oft nur als Orientierungshilfe für bestimmte häufige Probleme.

Quote:
€: Ich probier grade rum und muss sagen: das mit Vererbung wirkt gar nicht SOO doof. Meinungen?
Vererbung drückt eine "ist ein"-Beziehung aus. Wenn nun Main von allem erbt, dann _ist_ deine Main-Klasse Netzwerkcontroller, PacketHandler und Inventar (sollte sie aber nicht sein).
05/30/2013 12:07 Padmak#3
Danke, ähnliches hab' ich mir bereits gedacht.
Ich denke ich gehe eher in Richtung DI, sonst hab ich letztendlich so ewige Aufrufs-Ketten von Main->Network->PacketHandler->Main->Inventar etc, da ist das erste wesentlich eleganter und durchsichtiger

Jetzt muss ich mir nur noch überlegen, ob ich die Instanzen im Konstruktor oder in Setter-Methoden setze, ist aber egal oder?

Padmak
05/30/2013 12:25 Schlüsselbein#4
Quote:
Jetzt muss ich mir nur noch überlegen, ob ich die Instanzen im Konstruktor oder in Setter-Methoden setze, ist aber egal oder?
Jop, das spielt keine Rolle. Ausschlaggebend ist nur, dass sich das erzeugte Objekt selber nicht darum kümmern muss, woher die Klasse kommt, auf die es zugreifen muss.
05/30/2013 13:21 Padmak#5
Okay vielen Dank, dann weiß ich schon wie ichs machen muss. Somit vorerst erledigt, außer mir fällt was neues auf^^

Padmak
05/30/2013 13:59 dowhile#6
Mit Variante 2 wird Main zu einem Gottobjekt, einem Anti-Pattern.
Wieso das MVC sein soll, verstehe ich nicht so ganz.
05/30/2013 17:02 Schlüsselbein#7
Nein, es ist kein MVC. Tut mir leid, ich hatte mich etwas voreilig und unverständlich ausgedrückt.
Ich kenne natürlich nicht seinen kompletten Code und auch nicht seine anderen Klassen. Mit "in die Richtung MVC" meinte ich, dass seine Klasse mit dem Namen Main (etwas blöd gewählt in dem Fall), als Controller fungieren könnte. Dass es hier noch weiterer Arbeit bedarf, sollte klar sein.

Aber um mich nochmal zu wiederholen: Klammer dich nicht zu fest an Patterns, sie sollen eine Hilfe sein. Und praktisch umsetzen lassen sie sich auch nur selten so ideal, wie sie auf dem Papier aussehen. Jedes Problem hat normalerweile eigene individuelle Schwierigkeiten, die bei den idealen Patterns eben nicht berücksichtigt werden. Deshalb bieten sie eine gute Orientierungshilfe, aber keine scheu, die Patterns abzuwandeln. Besser, als dein Code so zu biegen, dass es nur mühseelig und mit Aufwand genau auf ein Muster passt.