@Muck22: Ist gibt ja eine Menge Konventionen gegen die man verstossen kann, aber `self` nicht `self` zu nennen ist etwas was man überhaupt nicht tun sollte. Da kommt auch echt kaum jemand drauf.
Das `os`-Modul wird importiert aber nicht verwendet.
Code auf Modulebene solle nur Konstanten, Funktionen, und Klassen definieren. Das Hauptprogramm gehört in eine Funktion. Und das vermischen von Hauptprogramm-Fragmenten mit Funktions- oder Klassendefinitionen macht das ganze noch mal ein Stück unübersichtlicher.
Wenn man einen Wahrheitswert meint sollte man keine Zahlen verwenden. Bei einer Zuweisung von 1 oder 0 fragt sich der Leser ob auch 2 oder 3 eine Bedeutung haben könnte, wogegen bei `True` oder `False` klar ist, dass 2 oder 3 eher nicht in Frage kommen.
Namen sollte nicht aus kryptischen Abkürzungen bestehen oder gar nur aus einzelnen Buchstaben, solange der Gültigkeitsbereich nicht sehr beschränkt ist oder die Abkürzung allgemein gebräuchlich ist. Es ist ja nicht so als wenn der Speicher knapp wäre oder man pro Buchstabe Geld bezahlen müsste das man gezwungen ist `e` statt `error` zu schreiben.
Das gleiche gilt auch für `message` statt `msg` oder `button` statt `but1`. Beim letzteren ist die Nummer ein Warnzeichen. Wenn man anfängt Namen zu nummerieren macht man in der Regel etwas falsch.
Die GUI-Elemente haben insgesamt schlechte Namen, insbesondere die, die als Attribute auf dem Objekt gesetzt werden, sollten dem Leser vermitteln was sie *bedeuten*. `tc` und `tc1` tun das nicht. Selbst `text_control` wäre ein schlechter Name weil man dann immer noch nicht weiss wozu dieses Texteingabefeld da ist. Zur Eingabe von Nachrichten? Zur Anzeige von Nachrichten? Anzeige von Fehlern? Irgend etwas anderes?
Warum die GUI-Klasse `Size` heisst, ist mir ein absolutes Rätsel.
Bei IDs würde ich statt -1 die Konstante `wx.ID_ANY` übergeben, dann weiss der Leser sofort was das Argument für eine Bedeutung hat.
Kommentare sollten einen Mehrwert zum Quelltext darstellen und dem Leser nicht erzählen *was* gemacht wird, das sieht er ja schon am Code, sondern *warum* der Code das macht was er macht, sofern das nicht ebenfalls einfach am Code ablesbar ist.
Das Layout regelt man in `wx` am besten über Sizer, *nicht* über absolute Grössen und Positionsangaben. Das kann man heutzutage nicht mehr machen ohne Gefahr zu laufen dass die GUI auf anderen Rechner als dem auf dem sie erstellt wurde, besch… aussieht oder gar unbenutzbar wird wenn Texte nicht mehr in den Platz passen oder so klein sind dass man sie nicht mehr vernünftig lesen kann. Die Zeiten das jedes Gerät einen Monitor hat dessen Auflösung und Masse ungefähr 96 DPI ergeben sind lange vorbei.
Beim `Bind()` ist es eher ungewöhnlich eine `id` für eine Schaltfläche zu übergeben. Man kann als drittes Argument einfach die Schaltfläche selbst als Argument angeben.
In Funktionen und Methoden sollte man ausser auf Konstanten auf nichts zugreifen was nicht als Argument übergeben wird. In `send()` darf man also nicht einfach auf `udp_sock` zugreifen was auf magische Weise irgendwo in der Umgebung zu existieren scheint.
Den Inhalt vom Texteingabefeld einfach mit `str()` in einen Bytestring zu wandeln wird dir um die Ohren fliegen sobald dort Zeichen ausserhalb des ASCII-Wertebereichs eingegeben werden. An der Stelle sollte explizit von Unicode nach Bytes kodieren. UTF-8 bietet sich als Kodierung an.
Bei dem Thread sollte man das `daemon`-Attribut auf `True` setzen, damit das Programm auch tatsächlich endet wenn das Fenster geschlossen wird, und nicht auf das Ende des Threads gewartet wird.
Die ``while not reader.running:``-Schleife verstehe ich nicht‽ Wozu soll die gut sein?
Über Threads hinweg sichere Aufrufe in `wx` bekommt man mit `wx.CallAfter()` hin. Ich habe mal die minimal nötigsten Änderungen in der Richtung am Programm vorgenommen, wobei die Aufteilung des Codes so nicht wirklich schön ist. Man sollte die Kommunikation zum Beispiel komplett in einer Klasse kapseln die weder Thread noch GUI ist und die Low-Level-Einzelteile wie Sockets nicht in der Gegend herum reichen.
Code: Alles auswählen
#!/usr/bin/env python
from __future__ import absolute_import, division, print_function
import socket
import threading
import wx
class Reader(threading.Thread):
def __init__(self, udp_sock, callback=lambda message: None):
threading.Thread.__init__(self)
self.udp_sock = udp_sock
self.callback = callback
self.running = False
def run(self):
self.running = True
while self.running:
try:
message, _ = self.udp_sock.recvfrom(8000)
self.callback(message)
except socket.error, error:
print(error)
class MainWindow(wx.Frame):
def __init__(self, parent, title, reader, udp_sock):
wx.Frame.__init__(self, parent, wx.ID_ANY, title, size=(450, 300))
self.reader = reader
self.udp_sock = udp_sock
self.SetIcon(wx.Icon('icons/wwww.ico', wx.BITMAP_TYPE_ICO))
self.SetBackgroundColour((238, 207, 161))
self.Centre()
self.Show(True)
send_button = wx.Button(self, wx.ID_ANY, 'senden', pos=(15, 90))
wx.StaticText(self, label="Nachricht eingeben!", pos=(180, 83))
self.text_input = wx.TextCtrl(
self, wx.ID_ANY, size=(150, 22), pos=(180, 100)
)
self.text_output = wx.TextCtrl(
self, wx.ID_ANY, size=(150, 22), pos=(180, 150)
)
wx.StaticBitmap(
self, wx.ID_ANY, wx.Bitmap('icons/send.png'), pos=(110, 83)
)
wx.StaticBitmap(
self, wx.ID_ANY, wx.Bitmap('icons/yyyy_logo.png'), pos=(15, 120)
)
self.Bind(wx.EVT_BUTTON, self.send, send_button)
self.reader.callback = self.receive
def send(self, _event):
message = self.text_input.GetValue()
self.udp_sock.sendto(message.encode('utf8'), ('localhost', 5555))
def receive(self, message):
wx.CallAfter(self.text_output.SetValue, message.decode('utf8'))
def main():
udp_sock = socket.socket(socket.AF_INET, socket.SOCK_DGRAM)
udp_sock.bind(("", 5555))
reader = Reader(udp_sock)
reader.daemon = True
reader.start()
app = wx.App()
MainWindow(None, 'xxxx Infotool', reader, udp_sock)
app.MainLoop()
if __name__ == '__main__':
main()
