Client-Server-Klassen für den Schulunterricht

Sockets, TCP/IP, (XML-)RPC und ähnliche Themen gehören in dieses Forum
Antworten
EmaNymton
User
Beiträge: 174
Registriert: Sonntag 30. Mai 2010, 14:07

Hallo zusammen,
ich hätte gerne eine Rückmeldung zu meinem Quelltext, den ich später so im Unterricht einsetzen möchte.

Vorher muss ich aber ein wenig ausholen ;)
Wir setzen seit diesem Schuljahr Python (vorher Java) bei uns im Unterricht ein und ich bin gerade dabei die von NRW vorgegebene Java-Klassen, die die Schüler im Abitur kennen sollen, in Python umzusetzen.

Die nächste Unterrichtsreihe behandelt Netzwerke, so dass ich gerade dabei bin, dafür Beispiele und Aufgaben zu erstellen. Natürlich starten wir erst mal mit den Grundlagen. Das ist mit dem Socket-Modul und kleineren Skripten ohne OOP auch kein Problem. Ich plane aber gerne rückwärts, so dass ich weiss, welche Grundlagen notwendig sein werden. Zum Abschluss der Reihe sollen die Schülerinnen und Schüler ein eigenes Projekt umsetzen, dass ein wenig anspruchsvoller ist, z.B. ein Netzwerkspiel (Schiffe versenken, TicTacToe, Vier gewinnt, ...) oder ein Chat-Programm.
Beim Zentralabitur mit Java ist die Vorgabe, dass eine Server- und eine Clientklasse zur Verfügung gestellt wird, die nachher als Oberklasse verwendet werden sollen. Die Java-Quelltexte findet man hier: http://www.standardsicherung.schulminis ... ?file=3183
(dort unter abiturklassen -> netzklassen)

Dieses Prinizip mit den zwei Oberklassen habe ich nun versucht in Python umzusetzen und habe ein Chat-Programm geschrieben. Die beiden Oberklassen:

client.py

Code: Alles auswählen

#!/usr/bin/env python
# -*- coding: utf-8 -*-

import socket
import sys
import threading

class Client(object):
    """ 
    Ein Client, der Nachrichten schicken kann und auf eingehnede
    Nachrichten wartet.
    Ankommende Strings werden nach unicode dekodiert und beim Versenden
    vorher wieder enkodiert.
    """ 
    def __init__(self):
        self.socket = socket.socket(socket.AF_INET, socket.SOCK_STREAM)

    def connect_to_server(self, ip, port):
        try:
            self.socket.connect((ip, port))
            print "Verbindung hergestellt."
        except socket.error, error:
            print "Fehler beim Verbindungsaufbau:", error
        try:
            thread = threading.Thread(target=self.receive_data, 
                                      args=[self.socket])
            thread.daemon = True
            thread.start()
        except KeyboardInterrupt:
            sys.exit()
        
    def receive_data(self, socket):
        while True:
            data = socket.recv(1024).strip().decode('utf-8')
            # Jeder Befehl vom Server endet mit '\n' 
            data_split = data.split('\n')
            if not data:
                self.socket.close()
                break
            for data in data_split:
                self.process_data(data)

    def send_data(self, data):
        try:
            self.socket.sendall(data.encode('utf-8'))
        except socket.error, error:
            print "Fehler beim Senden:", error

    def process_data(self, data):
        print data
server.py

Code: Alles auswählen

#!/usr/bin/env python
# -*- coding: utf-8 -*-

import socket
import threading

class Server(object):
    """
    Ein Server, der mehrere Clients bedienen kann. Alle empfangenen Daten
    werden per Default an den Client zurückgeschickt. 
    Die Funktionalität kann verändert werden, wenn die Methoden 
    process_data, process_new_connection und process_close_connection
    überschrieben werden.
    """
    def __init__(self, host='', port=9999):
        self.socket = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
        self.socket.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
        self.socket.bind((host, port))
        self.socket.listen(10)
        self.clients = []

    def _start(self):
        print "Server laeuft! Mit Strg+C Server stoppen!"
        while True:
            clientsocket, (host, port) = self.socket.accept()
            self._handle_connection(clientsocket, host, port)
        
    def _handle_connection(self, clientsocket, host, port):
        """
        startet für jeden neuen Client einen Thread, indem auf ankommende 
        Nachrichten des Clients reagiert werden kann.
        Der Client wird einer Liste aller verbundenen Clients angehängt"
        """
        try:
            thread = threading.Thread(target=self._connection_handler, 
                                      args=(clientsocket, host))
            thread.daemon = True
            thread.start()
            self.clients.append(clientsocket)
            self.process_new_connection(clientsocket)
        except (KeyboardInterrupt, SystemExit):
            for client in self.clients:
               self._disconnet_client(clientsocket)

    def _connection_handler(self, clientsocket, host):
        while True:
            data = clientsocket.recv(1024).strip()
            if not data:
                self._disconnect_client(clientsocket)
                break
            self.process_message(clientsocket, data.decode('utf-8'))

    def _disconnect_client(self, clientsocket):
        """handelt alle Vorgänge beim Abmelden eines Clients ab"""
        self.process_close_connection(clientsocket)
        self.clients.remove(clientsocket)
        clientsocket.close()

    def _send_data(self, clientsocket, data):
        """
        Vorgabe: An jede Nachricht wird '\n' angehaengt, 
        damit der Client weiss, dass der Befehl zu Ende ist.
        """
        try:
            clientsocket.sendall('{}\n'.format(data.encode('utf-8')))
        except socket.error, error:
            print "Fehler beim Versenden: {}".format(error)

    def _send_to_all(self, data):
        for clientsocket in self.clients:
            self._send_data(clientsocket, data)

    def process_new_connection(self, clientsocket):
        """in abgeleiteter Klasse ggf. überschreiben"""
        pass

    def process_close_connection(self, clientsocket):
        """in abgeleiteter Klasse ggf. überschreiben"""
        pass 

    def process_message(self, clientsocket, data):
        """default: Echo-Server, ggf. in abgeleiteter Klasse überschreiben"""
        self._send_data(clientsocket, data)

if __name__ == '__main__':
    server = Server()
    server._start()
Daraus mal als Beispiel eine Implementation eines etwas anspruchsvolleren Chat-Servers mit Namensanmeldung und der Möglichkeit, private Nachrichten zu schicken:

Code: Alles auswählen

#!/usr/bin/env python
# -*- coding: utf-8 -*-

from server import Server
from datetime import datetime

class ChatServer(Server):
    def __init__(self, host='', port=9999):
        Server.__init__(self, host, port)
        self.names = []
        
    def process_message(self, clientsocket, data):
        data = data.split()
        command = data[0]
        if command == 'NAME':
            name = "".join(data[1:])
            if name in self.names:
                self._send_data(clientsocket, "FAIL")
                self._disconnect_client(clientsocket)
            else:
                self._send_data(clientsocket, "OK")
                self.names.append(name)
                self._send_to_all(u"{} hat den Chat betreten.".format(name))
                self._send_to_all(u'{} {}'.format('LIST',' '.join(self.names)))
                print u"Verbindung von {} aufgebaut.".format(name)
        elif command == 'ALL':
            message = u' '.join(data[1:])
            name = self.names[self.clients.index(clientsocket)]
            time_now = str(datetime.now().strftime("%H:%M:%S"))
            self._send_to_all(u"({}) {}: {}".format(time_now, name, message))
            print u"{} schickt Nachricht an alle: {}".format(name, message)
        elif command == 'ONE':
            name = data[1]
            message = ' '.join(data[2:])
            time_now = str(datetime.now().strftime("%H:%M:%S"))
            sender_name = self.names[self.clients.index(clientsocket)]     
            data = u"({}) Private Nachricht von {}: {}".format(time_now, 
                                                               sender_name,
                                                               message)
            index = self.names.index(name)
            self._send_data(self.clients[index], data)
            print u"{} schickt private Nachricht an {}: {}".format(sender_name,
                                                                   name,
                                                                   message)
        elif command == 'QUIT':
            self._disconnect_client(clientsocket)

    def process_close_connection(self, clientsocket):
        try:
            name = self.names[self.clients.index(clientsocket)]
            self.names.remove(name)
            print u"Verbindung von {} beendet.".format(name)
            self._send_to_all(u"{} hat den Chat verlassen.".format(name))
            self._send_to_all(u'{} {}'.format('LIST',' '.join(self.names)))
        except IndexError:
            print u"Name nicht in der Liste"
            
if __name__ == '__main__':
    chatserver = ChatServer()
    chatserver._start()
Den Chat-Client (PyQt) mit ui-File findet man hier:
https://dl.dropboxusercontent.com/u/107 ... ient_qt.py
https://dl.dropboxusercontent.com/u/107 ... ient_qt.ui

zur Implementation:
Ich bin mir durchaus bewusst, dass die Implementation des "Protokolls" durch die if-elif Kaskadierung unschön ist und ggf. besser durch eine eigene Protokoll-Klasse gelöst werden sollte, jedoch orientiere ich mich da an den Vorgaben des Zentralabiturs, wo die Schüler im Endeffekt die process_message Methode überschreiben müssen. Die jeweilige Teilaufgabe im Abitur lautet dann, die für das Problem nötige process_message Methode zu schreiben.
Auch fehlen zum Teil Fehlerprüfungen.

Die Chat-Anwendung funktioniert, jedoch habe ich noch das Problem, dass beim Abmelden von Clients beim Server einen Fehler kommt, den ich so nicht zuordnen kann:

Code: Alles auswählen

Exception in thread Thread-2:
Traceback (most recent call last):
  File "/usr/lib/python2.7/threading.py", line 810, in __bootstrap_inner
    self.run()
  File "/usr/lib/python2.7/threading.py", line 763, in run
    self.__target(*self.__args, **self.__kwargs)
  File "/home/******/nas/*****/schule/Informatik/python/Skript Oberstufe/quelltext/netzwerk/server.py", line 47, in _connection_handler
    data = clientsocket.recv(1024).strip()
  File "/usr/lib/python2.7/socket.py", line 170, in _dummy
    raise error(EBADF, 'Bad file descriptor')
error: [Errno 9] Bad file descriptor
Bei meiner Suche im Netz habe ich viele Verweise gefunden, die daraufhin deuten, dass ein socket, der bereits geschlossen wurde, wiederverbunden werden soll. Das ist hier aber doch gar nicht der Fall?!

Außerdem wäre ich dankbar für jegliche Rückmeldungen bzgl. der beiden Oberklassen auch im Hinblick auf Verbesserungen, da es mit Sicherheit noch etwas zu verbessern gibt, mir aber auch die Erfahrung bzgl. Netzwerkprogrammierung fehlt.
Berücksichtigt bitte aber auch, dass Schüler die Klassen noch verstehen können sollen, deswegen würde ich sie gerne unnötig aufblähen und mich auf das notwendige reduzieren, so dass die oben beschriebenen Anwendungsfälle damit realisierbar sind. Ich weiss, dass ich damit das Rad teilweise neu erfinde und z.B. twisted eine gute Alternative wäre, jedoch möchte ich es den Schülern, die im Abitur die Java-Quelltexte verstehen sollen, nicht schwieriger machen als notwendig. Deswegen der Spagat mit eigenen Klassen.

Gruß EmaNymton
Sirius3
User
Beiträge: 17711
Registriert: Sonntag 21. Oktober 2012, 17:20

@EmaNymton: ich fange mal einfach von oben nach unten an, aufzuschreiben, was mir am Code auffällt.
Sockets werden normalerweise erstellt und gleich verbunden. Bei Dir wird der socket in __init__ erzeugt und erst in connect_to_server verbunden. Ich würde ja gar keine extra connect-Methode erstellen.
Was soll die KeyboardInterrupt-Exception? Das Programm irgendwo innerhalb einer Klassenmethode mit exit zu beenden, wo man es niemals erwarten würde ist überraschend, also schlecht. Der Fall, dass das wirklich eintritt ist zudem verschwinden klein, denn einen Thread zu starten dauert ja nicht ewig. Die Methode receive_data braucht das socket-Argument nicht, da socket schon ein Klassenattribut ist, also entweder oder, wobei die Methode beides mischt. Es sollte auch erst auf leeres data geprüft werden, bevor mit den Daten viel passiert. Die Methode hat einen schweren Bug. Mit recv kann zwischen 1 und 1024 bytes gelesen werden, es ist nicht sichergestellt, dass es sich hierbei um eine komplette Nachricht handelt, weil es sich bei TCP um ein Streamingprotokoll handelt. Insbesondere können unvollständige utf8-Kodierungen gelesen werden, was beim Decodieren zu einem Fehler führt, der den Lesethread abbricht und die Verbindung zum Server in einem undefinierten Zustand hinterläßt (Daten werden nicht mehr gelesen und verstopfen irgendwann den Schreibpuffer des Servers, der dann hängen bleibt). Das strip sollte demnach auch nach dem Decodieren stattfinden, weil im Allgemeinen (zwar nicht bei utf8, aber wir wollen ja keine Speziallösungen unterrichten) Leerzeichen vor und nach dem Decodieren nicht das selbe sein müssen.
Lösung des Problems: So lange lesen, bis ein Nachrichtendezeichen ('\n') gelesen wird und die Daten danach dekodieren und an process_data weitergeben.
Bei send_data: Fehler sollten vom Aufrufer bearbeitet werden (können) und nicht innerhalb der Methode in eine Ausgabe umgeleitet werden.
Beim Server: das Exceptionhandling in '_handle_connection' ist auch völlig überflüssig und unnötig, weil es nie auftritt. Warum wird client_handler clientsocket und host übergeben aber nicht port? Die recv-Schleife ist ebenso fehlerhaft wie beim Client. Warum fangen die meisten Methoden mit _ an? Insbesondere _start wird auch außerhalb der Klasse verwendet. Beim client werden die "Messages" bereits in der recv-Schleife getrennt, beim Server nicht.
Beispiel-Server: Namen und alle nachfolgenden Nachrichten, die zufällig in einem recv ankommt werden ohne Trenner (Leerzeichen?) zu einem Namen zusammengepappt. send_to_all sendet sowohl userdefinierte Namen als auch Kommandos (LIST) als ersten Teil einer Nachricht. Prinzipiell kann also jeder beliebige Commandos an alle anderen Clients schicken (Potentiell schweres Sicherheitsleck). str bei time_now ist überflüssig. Die Listen names und clients müssen synchron sein, damit sender_name funktioniert. Das ist ein schwerer Designfehler, der das Programmieren unnötig schwierig und fehleranfällig macht. Sollen zu Clients weitere Informationen gespeichert werden, wäre ein Dictionary für self.clients sinnvoll.
Fehler in process_message werden nicht abgefangen, so dass der Server eine Clientverbindung in einen undefinierten Zustand versetzen kann (reagiert nicht mehr) ohne Rückmeldung.

Die if-elif-Kaskade ersetzt man am Besten durch ein Dictionary-Lookup, würde man in Java wohl auch machen, wobei es da wegen der vielen Klassen, die man erzeugen müßte, ungleich komplizierter ist und man deshalb wohl eher geneigt ist, die if-elifs zu schreiben.

Zur Fehlermeldung: Du schließt in process_message den Clientsocket und versuchst danach wieder zu lesen.
Lösung: process_message gibt ein Flag zurück, ob es die recv-Schleife verlassen soll und der close-Socket findet erst nach der while-True-Schleife statt.

edit: so jetzt habe ich noch den Client-Code entdeckt. ChatClient bekommt den namen, verwendet ihn aber nicht. Dagegen weiß die GUI, wie sie den Namen an den Server schicken muß, was nicht gerade eine Trennung zwischen GUI- und Verarbeitungslogik ist. In process_message gibt es schöne Effekte, wenn sich User mit den Namen OK, FAIL oder LIST verbinden. In send_message_to_all wird join verwendet um Befehl und Nachricht miteinander zu verbinden. Dafür ist es nicht gedacht, nimm String-Formatierung dafür. So oft wie QT-Textfeldtext nach unicode convertiert wird, böte sich eine eigene Funktion an.

edit2: ich habe jetzt auch die Java-Beispielquelltexte angeschaut. Bei den Javaklassen haben sich auch einige schwere Fehler eingeschlichen. Die Schreiber sollten sich dringend informieren, wie man threadsicher über Listen iteriert. Bei der jetzigen Implementierung können an einigen Stellen unvorhersehbare Probleme entstehen, wenn mehrere Clients gleichzeitig Nachrichten senden.
Der Fehler mit halben Nachrichten durch das TCP-Streaming tritt jedoch nicht auf, weil die Sockets an einen InputStreamReader gebunden wird.
Eine einheitliche Namenskonvention fehlt, es gibt einbuchstabige Attribute und lustige Zeichen vor den Namen. Eigentlich dachte ich "l" bedeutet lokale Variable, aber in BinaryTree haben plötzlich alle Klassenattribute ein "l".
Eine weitere Variante, wie man GUI und Verarbeitung unglücklich verknüpfen kann, zeigt ja ChatClient, wo ein JTextArea direkt befüllt wird. Bei "hatChatClient" denke ich auch eher ein ein Boolean als an eine ChatClient-Klasse.
Von Materialien, die vom Schulministerium verteilt werden, erwarte ich eigentlich schon, dass sie von jemandem, der in Didaktik und Informatik geschult ist, gelesen und korrigiert wird, da jeder Fehler und jede Inkonsistenz hundertfach kopiert und wiederholt wird. Mich wundert manchmal was Schüler ihren Lehrern alles glauben und wie schwer es ist, ihnen z.B. hier im Forum das Falsche wieder aus ihren Köpfen zu bringen.
Zuletzt geändert von Sirius3 am Freitag 2. Mai 2014, 12:26, insgesamt 2-mal geändert.
BlackJack

@EmaNymton: So ganz grundsätzlich ist die Socketprogrammierung fehlerhaft. TCP ist ein Datenstrom und es ist nicht garantiert wie viel von den gesendeten Daten bei einem `recv()` ausgelesen werden. Wenn auf der Senderseite 'hallo\n' gesendet wird, auch bei einem `sendall()`, muss der Empfänger trotzdem im Extremfall damit klar kommen, dass das in *sechs* unterschiedlichen `recv()` gelesen und verarbeitet werden kann in denen jeweils nur ein Byte gelesen wird.

Warum ist `Server._start()` eine nicht-öffentliche Methode?

Bei der Verarbeitung der Chatserver-Nachrichten würde ich nicht die gesamte Nachricht splitten und dann bei fast jedem Befehl wieder zusammensetzen, sondern im ersten Schritt nur das Kommando vom Rest splitten. Das kann man mit ``split(None, 1)`` machen, aber vielleicht wäre die `partition()`-Methode auch geeigneter. Also zum Beispiel ``command, _, data = data.partition(' ')``.

Und dann könnte man vielleicht eine einfache Dispatch-Methode daraus machen um die ganzen ``if``/``elif``\s loszuwerden. So etwas wie ``getattr(self, 'do_command_' + command.lower())(data)``. Und dann einfach Methoden wie `do_command_name()` usw. implementieren.

Das `names` eine ”parallele” Liste zu den Sockets ist, die zusammen damit eigentlich wie ein Wörterbuch verwendet wird, finde ich ja irgendwie sehr unschön.
EmaNymton
User
Beiträge: 174
Registriert: Sonntag 30. Mai 2010, 14:07

Danke für die ausführlichen Antworten, das hat mir schon sehr viel gebracht!
Da habe ich am Wochenende ja doch noch was zu tun ;)

Einige Dinge (Exeption Handling, falsche Bezeichner) im Quelltext sind einfach vom Ausprobieren übrig geblieben, da ich weder mit Threads noch mit Netzwerk unter Python bisher sowas gemacht habe. Ich bin gerade dabei es zu bereinigen.
Bei Dir wird der socket in __init__ erzeugt und erst in connect_to_server verbunden. Ich würde ja gar keine extra connect-Methode erstellen.
Ich wollte einfach bei einem Einbau in eine GUI später die Möglichkeit haben, das Programm zu starten ohne dass der Client sich direkt verbindet. Ist aber auch eine Überlegung das direkt in die init-Methode zu setzen.
Die Methode receive_data [...]
Lösung des Problems: So lange lesen, bis ein Nachrichtendezeichen ('\n') gelesen wird und die Daten danach dekodieren und an process_data weitergeben.
Also quasi in der Form?

Code: Alles auswählen

 def receive_data(self):
        data_buffer = ""
        while True:
            data_buffer += self.socket.recv(1024)
            if not data_buffer:
                self.socket.close()
                break
            if data_buffer.endswith('\n'):
                data = data_buffer.decode("utf-8").strip()
                self.process_message(data)
                data_buffer = ""
Bei meinen Tests kam es dann aber immer noch vor, dass wenn z.B. zwei Befehle hintereinander geschickt wurden diese bei einem recv-Aufruf vollständig in data_buffer landeten, also z.B. "Alice hat den Chat betreten.\nLIST Bob Alice\n"
D.h. ich bin doch wieder darauf angewiesen, den erhaltenen String nach "\n" zu splitten, oder? Mir fällt da jetzt spontan keine andere Lösung ein.
Das `names` eine ”parallele” Liste zu den Sockets ist, die zusammen damit eigentlich wie ein Wörterbuch verwendet wird, finde ich ja irgendwie sehr unschön.
Ja, finde ich ja auch. Ich wollte die Clients im Server erstmal nur verwalten können. Da war mir eine Liste als einfache Datenstruktur erstmal naheliegend. Wenn ich die sockets jetzt über Namen ansprechen will, böte sich da ein dict an, wobei dieser Fall ja auch nicht immer gewünscht ist und eine Liste in vielen Fällen erstmal ausreicht, z.B. bei einem TicTacToe mit zwei Spielern. Deswegen der umweg über eine zweite Liste und der Index als Identifikationsmerkmal.
Von Materialien, die vom Schulministerium verteilt werden, erwarte ich eigentlich schon, dass sie von jemandem, der in Didaktik und Informatik geschult ist, gelesen und korrigiert wird, da jeder Fehler und jede Inkonsistenz hundertfach kopiert und wiederholt wird. Mich wundert manchmal was Schüler ihren Lehrern alles glauben und wie schwer es ist, ihnen z.B. hier im Forum das Falsche wieder aus ihren Köpfen zu bringen.
Da hast du leider Recht und ich habe es bisher auch relativ unreflektiert übernommen, wobei ich mich bei den Bezeichnern jedesmal aufrege, auch bei den anderen Klassen.
Ich denke das Problem liegt vor allem daran, dass viele Kollegen das Fach nicht wirklich studiert haben bzw. gerade programmieren nicht so intensiv betreiben, um die nötigen Kenntnisse/Erfahrungen zu haben. Gerade aus diesem Grund bin ich dankbar für dieses Forum, da ich hier zumindest eine Rückmeldung bekomme, wie viel Blödsinn in meinen Quelltexten drin steckt und wie ich es hoffentlich in Zukunft vermeiden kann. ;)

Ich weiß ehrlich gesagt auch nicht, wer für die Quelltexte letztendlich verantwortlich ist, aber vielleicht lohnt ja eine Mail ans Ministerium nach dem Motto: "Ich würde gerne meine Hilfe zur Verbesserung anbieten, sie dürfen sich gerne bei mir melden, wenn dazu Interesse besteht."
Sirius3
User
Beiträge: 17711
Registriert: Sonntag 21. Oktober 2012, 17:20

@EmaNymton: so etwa:

Code: Alles auswählen

    def receive_data(self):
        data_buffer = ""
        while True:
            data += self.socket.recv(1024)
            if not data:
                break
            data_buffer += data
            while '\n' in data_buffer:
                data, data_buffer = data_buffer.split('\n',1)
                data = data.decode("utf-8").strip()
                self.process_message(data)
        self.socket.close()
EmaNymton
User
Beiträge: 174
Registriert: Sonntag 30. Mai 2010, 14:07

Hallo,
hat doch noch etwas länger gedauert, da am Wochenende anderes auf dem Plan stand. Ich habe, soweit ich das richtig sehe, eure Anmerkungen/Vorschläge eingepflegt und das Protokoll dahingehend abgeändert, dass immer zuerst ein Befehl gesendet wird und dann die dazu nötigen Daten. Außerdem habe ich die Trennung zwischen GUI und ChatClient deutlicher gemacht, in dem die Daten, die beim ChatClient ankommen auch dort verarbeitet werden und dann per Signal an die GUI gesendet werden. Ist imho sinnvoller als es vorher war, wo die GUI diese Aufgabe mit übernommen hatte.

Ich würde mich sehr freuen, wenn ihr nochmal einen Blick drüber werfen könntet. Meine Tests verlaufen soweit positiv, doch wie ich mich kenne, habe ich bestimmt noch was übersehen ;)

https://dl.dropboxusercontent.com/u/107743837/client.py
https://dl.dropboxusercontent.com/u/107743837/server.py
https://dl.dropboxusercontent.com/u/107 ... ient_qt.py
https://dl.dropboxusercontent.com/u/107 ... ient_qt.ui
https://dl.dropboxusercontent.com/u/107 ... _server.py
EyDu
User
Beiträge: 4881
Registriert: Donnerstag 20. Juli 2006, 23:06
Wohnort: Berlin

Es wäre etwas schöner, wenn du dein Projekt als Repository bereitstellst. Zum Beispiel bei GitHub. Das lässt sich dann etwas schöner lesen. Und da du zum Entwickeln bestimmt schon ein Versionsverwaltungssystem einsetzt, sollte das für dich keinen Aufwand darstellen ;-)

Ansonsten ist mir gleich erstmal aufgefallen, dass ``Client.receive_data`` und ``Server._connection_handler`` quasi identisch sind. Das selbe gilt für die send_data-Methoden.

Ansonsten: Wie gehst du mit Daten um, welche "\n" enthalten?
Das Leben ist wie ein Tennisball.
Sirius3
User
Beiträge: 17711
Registriert: Sonntag 21. Oktober 2012, 17:20

Die Backslashs für Zeilenumbrüche sind alle überflüssig. Ich würde diese Möglichkeit, lange Zeilen umzubrechen auch gar nicht zeigen, da ich Klammern immer für Lesbarer halte. Die Index-Zugriffe im Server für Name und Client halte ich immer noch für sehr fehleranfällig, da die Listen an verschiedenen Stellen verwaltet werden. Niemand kann garantieren, dass sie bei jeder Änderung immer synchron sind. Das ganze Chat-Protokoll ist auch noch nicht gekapselt genug. Die GUI muß wissen, wie Command und Daten übertragen werden. Das Zusammenbauen der Nachrichten findet an sehr vielen Stellen im Client und im Server statt. Genauso das Lesen der Nachrichten ist doppelt. Das Aufsplitten findet wieder in einer viel zu tiefen Ebene statt, nämlich in der abgeleiteten Client- bzw. Server-Klasse. Ich würde zur Server-Klasse eine weitere Handler-Klasse schreiben, die die Anfragen eines Clients bearbeitet (ähnlich dem BaseHTTPServer). Die Fehlerbehandlung ist, insbesondere bei den process-Methoden, noch ungenügend. Dass manche Fehler abgefangen und über print ausgegeben werden, zeigt im Zusammenspiel mit einer GUI deutlich, warum so ein Vorgehen problematisch ist.
Die langen if-Kaskaden sind auch untypisch für Python, da es eine dynamische Sprache ist. Eine saubere Lösung könnte so aussehen:

Code: Alles auswählen

class Handler(object):
    def __init__(self, socket, server):
        self.socket = socket
        self.server = server

    def send_data(self, command, data=''):
        """
        Vorgabe: command darf nur Buchstaben enthalten, data darf keinen Zeilenumbruch enthalten.
        Eine Nachricht besteht aus command ' ' data '\n'.
        """
        assert command.isalpha() and '\n' not in data
        self.socket.sendall('{} {}\n'.format(command.upper(), data).encode('utf-8'))

    def process_message(self, data):
        command, _, data = data.partition(' ')
        return getattr(self, '_process_%s' % command.lower())(data)

class ChatHandler(Handler):
    def __init__(self, socket, server):
        Handler.__init__(self, socket, server)
        self.name = None

    def _process_name(self, name):
        if name in (client.name for client in self.server.clients):
            self.send_data("FAIL")
        else:
            self.name = name
            self.send_data("OK")
            self.server.send_to_all("MSG", "{} hat den Chat betreten.".format(name))
            self.server.send_to_all("LIST", ' '.join(self.names))
            print u"Verbindung von {} aufgebaut.".format(name)
        return True
Antworten