3. IRC-Bot Klassendesign

IRC-Bot Klassendesign

  • ?

    volkard schrieb:

    314159265358979 schrieb:

    Also du würdest für jedes Event einen Handler-Typen einführen?

    Nein, im Gegenteil. Ich dachte eher an einen Observer, der praktisch nur hereinkommende Zeilen observt und selber zerlegt und dispatscht. Observer? Nur, weil ich nicht ervben wollte. Fühlt sich auch an wie ein Dilemma.
    Aber ich bin auch nicht völlig sicher. Müßte die Klassen und Methodenprototypen mal eintippen und ein Bierchen trinken.

    Ich finde die Idee recht gut und lässt sich auch gut implementieren. Die Frage bei der Zerlegung ist allerdings, ob du die Zerlegung der Nachricht durch die Subscriber erfolgt oder ob dies bereits vom Observer erledigt wird. Ich sehe Vorteile in beiden Varianten.

    Vorteil Zerlegung im Observer:
    - Alle Subscriber können auf die zerlegten Daten zugreifen
    - Kein Overhead durch ständige Neu-Zerlegung

    Nachteil
    - Unflexibel bei unterschiedlichen Nachrichtenaufbau

    ---

    Vorteile Zerlegung im Subscriber
    - Kann auf jede Nachricht reagieren, da Kontrolle darüber behalten wird

    Nachteil
    - Viel Overhead durch ständige Neu-Zerlegung

    Was haltet ihr davon?

    #include <iostream>
#include <vector>
#include <string>
#include <sstream>

class Subscriber
{
	public:
		virtual void Update(const std::string& line, class Observer& observer) = 0;
};

class Observer
{
	typedef std::vector<Subscriber *> Subscribers;

	Subscribers subscribers;

public:
	~Observer()
	{
		for (Subscribers::iterator it = subscribers.begin(); it != subscribers.end(); ++it)
		{
			delete *it;
		}
	}

	void Notify(const std::string& line)
	{
		for (Subscribers::iterator it = subscribers.begin(); it != subscribers.end(); ++it)
		{
			(*it)->Update(line, *this);
		}
	}

	void Add(Subscriber *sub)
	{
		subscribers.push_back(sub);
	}
};

// 

class PingSubscriber : public Subscriber
{
	virtual void Update(const std::string& line, class Observer& observer)
	{
		std::string buf = line;

		if (buf[1] == 'I')	
		{
			buf[1] = 'O';

			std::cout << "Got PING!\nResponse: " << buf << "\n" << std::endl;
		}
	}
};

class PrivmsgSubscriber : public Subscriber
{
	// :Macha!~macha@unaffiliated/macha PRIVMSG #botwar :Test response
	virtual void Update(const std::string& line, class Observer& observer)
	{
		std::stringstream strm(line);

		std::string buf;

		std::vector<std::string> tmp;

		while (strm >> buf) 
			tmp.push_back(buf);

		if (tmp.size() >= 3 && tmp[1] == "PRIVMSG")
		{
			std::cout << "User: " << tmp[0] << std::endl;

			std::cout << "Channel: " << tmp[2] << std::endl;

			std::cout << "Message: ";
			if (tmp[3][0] == ':')
			{
				tmp[3].erase(0, 1);
			}

			for (std::size_t i = 3; i < tmp.size(); ++i)
			{
				std::cout << tmp[i] << " ";
			}

			std::cout << "\n" << std::endl;
		}
	}
};

int main()
{
	Observer observer;

	observer.Add(new PingSubscriber);
	observer.Add(new PrivmsgSubscriber);

	observer.Notify("PING bla");
	observer.Notify("PING bla");
	observer.Notify("PING :bla");
	observer.Notify(":User PRIVMSG #Channel :Response mit mehreren Woertern");
	observer.Notify(":User PRIVMSG #Channel2 :Response mit noch mehrer Woertern");
	observer.Notify(":User PRIVMSG #Channel3 Test!");
}
    0
  • 3

    Ich kenne dieses Pattern leider überhaupt nicht, daher fange ich damit nicht viel an. Gibts dazu irgendwelche hilfreichen Links?

    0
  • ?

    Das Pattern siehst du doch hier im Forum als Code.

    Ansonsten: http://de.wikipedia.org/wiki/Observer_(Entwurfsmuster)

    0
  • I

    Ich würde es auch besser finden, wenn die Zeile nicht ständig neu geparst werden muss. Dann schleichen sich nämlich schnell solche Nachlässigkeiten wie bei jhkhjkhjk ein, wo statt nur auf PING auch auf DING, BINGO oder bIber reagiert wird und die Privmsg-Funktion doppelte Leerzeichen in der Nachricht verschluckt.

    Möglicherweise bekommt man mit ein bisschen aufwendiger Metaprogrammierung so etwas hin (ich zeig mal nur die ungefähre Verwendung):

    void ban(session& s, string nick, string channel, int seconds, string reason);

int main()
{
    session s;

    cmdprocessor cmd(s);
    cmd.register_cmd("!ban", ban);    // "!ban ipsec #c-plusplus.net 12345 ohne grund" -> ban(s, "ipsec", "#c-plusplus.net", "ohne grund");
}
    0
  • 3

    Nach euren Anregungen habe ich mir nun folgendes Design überlegt:

    irc::socket
    - Ein gekapselter Socket von boost ohne viel weitere Funktionalität
    - Bietet 2 Funktionen read_line() und write_line(), die \r\n entfernen bzw hinzufügen.
    - Kennt keine Informationen über die session, also keinen nickname, auth, channel o.ä.

    irc::session
    - Bekommt im Konstruktor einen irc::socket, sowie für die Session relevante Daten (Nickname, Auth, Realname, evtl Passwörter, etc..)
    - Hat alle Daten gespeichert über Channels, User, etc..
    - Beantwortet Ping-Anfragen
    - Bietet Funktionen read_line(), write_line(), read_line(const std::string& chan) und write_line(const std::string& chan) = blockierende, thread-safe Funktionen

    irc::protocol
    - Ist für die irc-spezifischen Befehle verantwortlich
    - Ist eine Ansammlung von statischen Funktionen, wie z.B. make_nick_string(const std::string&)

    irc::event
    - (Abstrakte) Basisklasse für alle Arten von Events
    - Hat keine Funktionen

    irc::*_event
    - Konkrete Eventklassen
    - Im Konstruktor wird ein string übergeben und geparst
    - Bietet Zugriffsfunktionen auf die Daten, wie .new_nick(), .old_nick(), etc.

    irc::event_manager
    - Verwaltet die events
    - Verantwortlich für die event-Handler
    - Kann Handler sowohl channel-lokal, als auch global hinzufügen
    - Enthält enum event_type, das den Typ des Events angibt.
    - Ebenfalls thread-safe

    irc::event_handler
    - Basisklasse für event-Handler
    - nicht abstrakt
    - enthält für jedes Event bereits eine Funktion, z.B. on_nick_change()
    - Vordefinierte Funktionen enthalten nur ein throw method_not_overwritten_error("some text"), falls ein Handler mit einem Event registriert wurde, dessen Funktion er nicht überschreibt

    Sagt mir mal, ob das so brauchbar ist. 🙂

    0
  • ?

    ipsec schrieb:

    Ich würde es auch besser finden, wenn die Zeile nicht ständig neu geparst werden muss. Dann schleichen sich nämlich schnell solche Nachlässigkeiten wie bei jhkhjkhjk ein, wo statt nur auf PING auch auf DING, BINGO oder bIber reagiert wird und die Privmsg-Funktion doppelte Leerzeichen in der Nachricht verschluckt.

    Möglicherweise bekommt man mit ein bisschen aufwendiger Metaprogrammierung so etwas hin (ich zeig mal nur die ungefähre Verwendung):

    void ban(session& s, string nick, string channel, int seconds, string reason);

int main()
{
    session s;

    cmdprocessor cmd(s);
    cmd.register_cmd("!ban", ban);    // "!ban ipsec #c-plusplus.net 12345 ohne grund" -> ban(s, "ipsec", "#c-plusplus.net", "ohne grund");
}

    Das könnte schwierif werden, da Befehle bekanntlich unterschiedliche Parameter annehmen. Daher wäre es sinnvoller, wenn die Parameter als std::vector übergeben werden.

    0
  • A

    Sieht eigentlich gut aus. Denk auch dran, dass irc::session auch threadsicheren Zugriff auf die Channel- und Userinformationen bieten sollte.
    Bei mir sieht das Design sehr ähnlich aus, allerdings heißen die event_handler Plugins und der event_manager ist bei mir dementsprechend der plugin_manager.

    Ein Pluginsystem würde ich auf jeden Fall empfehlen. Ich habe über die Jahre auch eine ganze Reihe von IRC-Bots geschrieben und erst mit dem letzten (zum ersten Mal mit Pluginsystem) bin ich jetzt schon seit längerer Zeit zufrieden.
    Dank zur Laufzeit ladbarer Plugins kann man Funktionalität wie Logging, verschiedene Channelservices, Spiele, Remoteaccess, FServ, GUI-Client etc. jederzeit aktualisieren, entfernen oder hinzufügen, ohne dass man den Betrieb in den Channels durch Neustarten des Bots aufhalten müsste.

    0
  • 3

    So ein Pluginsystem ist auf alle Fälle geplant, allerdings durch die schon angesprochenen Python-Scripte. C++ Plugins sind nicht geplant, da ich dann keine Plattformunabhängigen Plugins schreiben kann.

    0
  • 3

    Ich habe mir noch folgendes dazu überlegt:
    irc::command_handler
    - Spezieller Handler-Typ für Befehle
    - Hat nur die Methode on_command
    - Wird beim event_manager, der auf plugin_manager umbenannt wird mit einer extra Funktion registrert

    0
  • 3

    Ich sitze gerade an der Implementierung des event_managers und bin etwas verzweifelt. Und zwar implementiere ich gerade die Funktionen read_line(), put_back() und run(). Alle 3 Funktionen müssen thread-safe sein.

    read_line() Liest eine Zeile und gibt diese zurück.
    put_back(const std::string&) Gibt eine Zeile zurück in den Buffer.
    run() liest laufend Daten ein, schreibt in den Buffer und informiert anschließend andere Threads über vorhandene Daten.

    Ich habe eine std::liststd::string als Buffer, einen boost::mutex und eine boost::condition_variable.
    (Die Variablen, die mit global_ anfangen sind nicht global, sondern heißen so, weil sie für "globale" IRC Nachrichten verwendet werden)

    std::string irc::event_manager::read_line()
{
	boost::unique_lock<boost::mutex> lock(global_mutex);

	while(global_buffer.empty())
		global_condition.wait(lock);

	std::string str = std::move(global_buffer.front());
	global_buffer.pop_front();
	return std::move(str);
}

void irc::event_manager::put_back(const std::string& line)
{
	boost::lock_guard<boost::mutex> lock(global_mutex);
	global_buffer.push_back(line);
	global_condition.notify_one();
}

void irc::event_manager::run()
{
	for(;;)
	{
		std::string line = session.read_line();
		{
			boost::lock_guard<boost::mutex> lock(global_mutex);
			global_buffer.push_back(std::move(line));
			global_condition.notify_one();
		}
	}
}

    Ich kenne mich mit Conditions noch kaum aus und wollte daher wissen, ob dieser Code korrekt sein kann. Gibt es vielleicht einen schönen Weg nur mit Mutexes?

    0
  • 3

    möp

    0
  • 3

    Niemand hier, der die Muse hat, sich mit hässlichem Code ala PI zu beschäftigen? Oder ist mein Post einfach unvollständig / fehlt irgendwas?

    0

  • Code sieht OK aus.

    Die Art und Weise wie du die Condition-Variable + Mutex verwendest ist auch OK, das ist genau so "wie mans macht". Schöneren Weg gibt es da keinen.

    Was anderes noch:

    314159265358979 schrieb:

    void irc::event_manager::put_back(const std::string& line)
{
	boost::lock_guard<boost::mutex> lock(global_mutex);
	global_buffer.push_back(line);
	global_condition.notify_one();
}

    Wenn du schon std::move im restlichen Code verwendest, wieso dann nicht auch hier?

    Nimm den Parameter by-value, und "move" den Parameter dann in die Liste hinein:

    void irc::event_manager::put_back(std::string line)
{
	boost::lock_guard<boost::mutex> lock(global_mutex);
	global_buffer.push_back(std::move(line));
	global_condition.notify_one();
}

    Wenn put_back ohne "move" aufgerufen wird, wird der String 1x kopiert, nämlich beim "by-value" übergeben. Bei global_buffer.push_back wird jetzt nur mehr gemoved, d.h. das ist billig.
    Ist also in dem Fall etwa gleich schnell wie deine Version (die immer genau 1x kopiert, nämlich bei global_buffer.push_back ).

    Wenn put_back aber MIT "move" aufgerufen wird, ist "meine" Variante schneller, da dann gar keine Kopie mehr gemacht wird.

    0
  • 3

    Vielen Dank für den Tipp! 🙂

    Was mir noch nicht ganz klar ist: Wieso brauche ich hier diese while-Schleife? (Ich hab mir die Verwendung aus der boost-Doku abgeguckt und verstehe deshalb noch nicht so ganz, was da abgeht.)

    0
  • ?

    314159265358979 schrieb:

    Was mir noch nicht ganz klar ist: Wieso brauche ich hier diese while-Schleife? (Ich hab mir die Verwendung aus der boost-Doku abgeguckt und verstehe deshalb noch nicht so ganz, was da abgeht.)

    http://en.wikipedia.org/wiki/Spurious_wakeup

    0
Anmelden zum Antworten