Meinung (Tipp) zu erstelltem Programmcode (TCP-Server)

Wenn du dir nicht sicher bist, in welchem der anderen Foren du die Frage stellen sollst, dann bist du hier im Forum für allgemeine Fragen sicher richtig.
Antworten
dkell
User
Beiträge: 12
Registriert: Mittwoch 12. März 2008, 09:40

Hallo

Ich erstelle ein kleines Programm, welches

* auf eine Schnittstelle lauscht und ankommende Daten an eine Queue übergibt (IRTrans)
* eine TCP/IP Server startet und ebenfalls auf ankommende Befehle lauscht und an die Queue übergibt
* ein Programmteil, welcher die Daten in der Queue verarbeitet

Da ich noch ziemlich Anfänger bin wäre es toll, wenn ein Python-Profi kurz über den Programmcode schauen könnte ob dieser Code-Teil so Sinn macht oder was besser gemacht werden sollte.

Des weiteren habe ich nicht rausgefunden, wie im im Teil SingleTCPHandler .. def handle() die Queue füllen kann.

Wenn mir hier auch jemand einen Tipp geben könnte.

Vielen dank

Code: Alles auswählen

class Trigger(threading.Thread):
    # Dieser Teil reagiert auf die Daten in der Queue (events) und arbeitet
    # diese ab

    def __init__(self, events):                # neuer Konstruktor
        threading.Thread.__init__(self)      # Aufruf des ererbten Konstruktors
        self.events = events

    def run(self):
        while True: 
            a = self.events.get()
            print "Wert in Queue: ", a
            if a == "irtrans.vcr0081.4":
                test()
            self.events.task_done()


##########################################################################
# TCP/IP-Server
########################################################################## 

class SingleTCPHandler(SocketServer.BaseRequestHandler):
    "One instance per connection.  Override handle(self) to customize action."

    def handle(self):
        # self.request is the client connection
        data = self.request.recv(1024)  # clip input at 1Kb

        cmd = data.strip()
        print cmd
        reply = "Antwort an Server; noch zu programmieren"
        #Trigger.events.put("Neuer Wert in Queue")
        
        if reply is not None:
            print "Rückgabewert: ", reply
            self.request.send(reply + '\n')
        self.request.close()


class SimpleServer(SocketServer.ThreadingMixIn, SocketServer.TCPServer):
    print "SimpleServer aufgerufen"
    # Ctrl-C will cleanly kill all spawned threads
    daemon_threads = True
    # much faster rebinding
    allow_reuse_address = True

    def __init__(self, server_address, RequestHandlerClass):
        SocketServer.TCPServer.__init__(self, server_address, RequestHandlerClass)


class Start_TCPServer(threading.Thread):

    def __init__(self):
        threading.Thread.__init__(self)

    def run(self):
        server = SimpleServer((HOST, TCP_PORT), SingleTCPHandler)
        try:
            print "TCP/IP-Server gestartet, warte auf Verbindungen"
            server.serve_forever()
        except:
            server.shutdown()
            print "TCP/IP-Server heruntergefahren"
        

#########################################################################
# AUFRUFE / START
#########################################################################

# Trigger für Verarbeitung der queue
print "... starte event-trigger"
trigger = Trigger(events)
trigger.start()


# auf IRTrans-Befehle lauschen
print "... lausche auf IRTRans Server"
irreceive = hc_irtrans.Irtrans_listen(events)
irreceive.start()


# TCP/IP Server starten
tcpserver = Start_TCPServer()
tcpserver.start()

[/list][/quote]
EyDu
User
Beiträge: 4881
Registriert: Donnerstag 20. Juli 2006, 23:06
Wohnort: Berlin

Hallo!

Ich schreibe meine Vorschläge mal so, wie sie mir aufgefallen sind. Die Reihenfolge hat also nichts mit Relevanz zu tun:

- Wenn du beschreibst, was eine Klasse oder eine Methode machen, dann solltest du Doc-Strings benutzen. In Zeile 24 machst du das ja auch, aber Zeilen 3 und 4 sollten entsprechend angepasst werden.
- Kommentar in der selben Zeile wie Code sind sehr unschön. Man kann sie kaum lesen, es sieht alles sehr durcheinander aus und wenn man den Code mal anpasst, dann muss immer die Kommentareinrückung angepasst werden. Schreibe also besser Kommentare über die entsprechende Stelle
- Ich würde mal vermuten, dass du in der handle-Methode noch Fehlerbehandlung benötigst.
- Ich habe es nicht nachzählen lassen, aber eine Zeile sollte nicht breiter sein als 79 Zeichen. Bei dir sieht die ein oder andere aber etwas länger aus.
- Der Aufruf in Zeile 14 führt wohl ins Leere.
- Die 1028 in Zeile 28 ist eine Konstante und sollte nicht einfach als magische Zahl mitten im Code auftauchen.
- Zeilen 48 und 49 kannst du dir sparen, wenn du in einer anderen Reihenfolge erbst
- Die init-Methode von Start_TCPServer, welches besser StartTCPServer heißen sollte, ist ebenfalls komplett überflüssig.
- Am besten aber, du sparst die die ganze Start_TCPServer-Klasse und ersetzt sie durch eine Funktion.
- Ein except, ohne dass die abzufangnen Fehler angegeben werden, ist eine ganz schlechte Idee. Es werden dann tatsächlich alle Meldungen verschluckt und das macht eine Fehlersuche unter umständen nahezu unmöglich.
- Code sollte nie auf Modul-Ebene stehen, dann kannst du das Modul nicht mehr importieren, ohne dass es ausgeführt wird. Packe also alles abe Zeile 71 in eine main-Funktion und rufe sie dann so auf:

Code: Alles auswählen

if __name__ == "__main__":
    main()
Und noch zu deinem Warteschlagen-Problem: http://docs.python.org/library/queue.html

Bis dann,
Sebastian
Das Leben ist wie ein Tennisball.
Benutzeravatar
jbs
User
Beiträge: 953
Registriert: Mittwoch 24. Juni 2009, 13:13
Wohnort: Postdam

[url=http://wiki.python-forum.de/PEP%208%20%28%C3%9Cbersetzung%29]PEP 8[/url] - Quak!
[url=http://tutorial.pocoo.org/index.html]Tutorial in Deutsch[/url]
dkell
User
Beiträge: 12
Registriert: Mittwoch 12. März 2008, 09:40

Vielen dank EyDu für die Tipps; werde mal versuchen das ganze ein wenig umzubauen.

jbs, das mit der Queue habe ich schon mal teilweise hin bekommen und funktioniert einwandfrei, hänge aber da noch an einem Ecken. Muss mich da noch etwas tiefer einlesen. Vielen dank
dkell
User
Beiträge: 12
Registriert: Mittwoch 12. März 2008, 09:40

Hi Sebastian, ich hätte mal noch 2 Fragen zu Deinen Bemerkungen
EyDu hat geschrieben: - Die 1028 in Zeile 28 ist eine Konstante und sollte nicht einfach als magische Zahl mitten im Code auftauchen.
Du meinst die recv(1024), oder? Was meinst Du mit "sollte nicht einfach im Code auftauchen"? Den Wert in eine Variable packen und so einbinden?
EyDu hat geschrieben: - Zeilen 48 und 49 kannst du dir sparen, wenn du in einer anderen Reihenfolge erbst
Was ist damit gemeint mit dieser "anderen Reihenfolge erbst" ?

Den Rest habe so mal hinbekommen

Vielen dank
Dani
Leonidas
Python-Forum Veteran
Beiträge: 16025
Registriert: Freitag 20. Juni 2003, 16:30
Kontaktdaten:

dkell hat geschrieben:
EyDu hat geschrieben: - Zeilen 48 und 49 kannst du dir sparen, wenn du in einer anderen Reihenfolge erbst
Was ist damit gemeint mit dieser "anderen Reihenfolge erbst" ?
Erst von ``SocketServer.TCPServer``, dann von ``SocketServer.ThreadingMixIn``.
My god, it's full of CARs! | Leonidasvoice vs (former) Modvoice
Benutzeravatar
cofi
Python-Forum Veteran
Beiträge: 4432
Registriert: Sonntag 30. März 2008, 04:16
Wohnort: RGFybXN0YWR0

dkell hat geschrieben:Du meinst die recv(1024), oder? Was meinst Du mit "sollte nicht einfach im Code auftauchen"? Den Wert in eine Variable packen und so einbinden?
Ja, `LENGTH = 1024; recv(LENGTH)` macht die Bedeutung der Zahl klarer.
dkell hat geschrieben:Was ist damit gemeint mit dieser "anderen Reihenfolge erbst" ?
Wenn du kein `__init__` schreibst, wird die Methode der 1. Superklasse benutzt und da du nur die Methode der 2. aufrufst, kannst du die beiden Superklassen vertauschen und `__init__` weglassen.
Antworten