@TrayserCassa: Der Sternchen-Import ist in der Tat unschön.
Es gibt 15 Docstrings die allesamt komplett für die Tonne sind weil sie nichts dokumentieren.
In der `__init__()` einer Klasse ein übergebenes Containerwidget mit Widgets zu bestücken ist mindestens komisch. Das ist unflexibel. Normalerweise erzeugt man dieses Containerwidget selbst bzw. normalerweise ist die Klasse dieses Widget und der Aufrufer/Erzeuger kann dann entscheiden wie und wo er das dann in eine GUI einbaut.
Ein paar von den Methodennamen sind für meinen Geschmack zu kurz. `send()` beispielsweise könnte durchaus auch verraten *was* da gesendet wird. Denglisch wie bei `show_hilfe()` ist auch nicht schön. Insgesamt sollte man bei einer Sprache für die Bezeichner bleiben. Das gilt auch für Texte die dem Benutzer angezeigt werden. Das denglische „eingeloggt“ heisst auf Deutsch „angemeldet“.
Wie Sirius3 ja schon angemerkt hat macht es keinen Sinn das Datenbankmodul in der `Dbms.__init__()` als Argument zu übergeben, dann aber sonst `pymysql` direkt anzusprechen und Sachen zu verwenden die nicht Standard sind. Und wieso heisst das Argument `root`? `Dbms` ist für dieses spezielle Objekt auch ein zu generischer Name. Der Host und der Datenbankname wären bessere Kandidaten für Argumente bei dieser Klasse als das Datenbankmodul.
Die `Dbms.list_*()`-Methoden sind nicht gut benannt. Da wird nicht wirklich etwas gelistet (was bedeutet das überhaupt) und `list_name()` liefert nicht *einen* *Namen* sondern *alle* Schülerdatensätze.
Insbesondere in der `Dbms`-Klasse werden zu viele triviale Zwischenergebnisse ohne Grund an einen Namen gebunden. Die ganzen `result`\s kann man sich locker sparen ohne dass das einen Einfluss auf die Lesbarkeit oder Verständlichkeit hätte.
In `count_tickets_payed()` steht Code *nach* dem ``return`` was unsinnig ist weil der *niemals* ausgeführt wird.
Der Name `user` wird in der `login()`-Methode als Schleifenvariable verwendet und *in* der Schleife dann an einen anderen Wert gebunden.
Der Statustext wird in der `login()` nach dem befüllen der Benutzerlistbox auf 'Wähle einen User aus der Liste aus.' gesetzt, nur um gleich danach auf 'Sie sind angemeldet.' geändert zu werden. Texte die der Benutzer nie zu sehen bekommt braucht man auch nicht “anzeigen“. Ein ähnliches Vorgehen ist in `schulden()` zu finden.
`notpayed` ist kein englisches Wort. Die Methode hiesse besser `count_unpaid_tickets()`. Allerdings wäre eine `get_payment_statistics()` die Anzahl und Beträge von bezahlten und unbezahlten Tickets abfragt vielleicht besser als zwei Anfragen zu stellen die auf die gleichen Spaltenwerte filtert. An der Stelle sollte man vielleicht auch den Spaltentyp von `bezahlt` auf BOOLEAN ändern statt Buchstaben zu verwenden wo dazu auch noch in einer Deutsch benannten Spalte die Buchstaben für englische Ja/Nein-Antworten stehen.
Das mit den Daten in der Listbox ist äusserst unschön gelöst. Man steckt keine Daten in eine Zeichenkette um die dann da umständlich hinterher wieder heraus zu prokeln. Dabei gehen Typinformationen verloren. Falls das tatsächlich klappt die einen Schüler auszuwählen und dann dessen Daten von Datenbank abzufragen dann hast Du an zwei Stellen unwahrscheinliches Glück. 1. das die DB dann als ID wo (hoffentlich) eine Zahl erwartet wird auch eine Zeichenkette akzeptiert, und 2. das die ID in Deinen Tests einstellig war, denn wenn sie mehr als ein Zeichen hat wird sie so wie Du `execute()` damit aufrufst das als mehr als einen Wert ansehen.
Das mit den Listboxen löst man in der Regel so dass man die Daten zu der Zeichenkette in einem Wörterbuch hinterlegt.
Der Test ob das `result` eine andere Länge als 0 hat vor der Schleife ist überflüssig. Das Programm verhält sich ohne den Test identisch und man spart sich eine Einrückebene. Und den Namen `result` kann man sich dann auch sparen.
Die Methoden von `Dbms` folgen alle einem Muster: Cursor erstellen, abfrage absetzen, alle Ergebnisse holen. So etwas kann man prima in eine eigene Methode auslagern.
Mir fehlt insgesamt so ein bisschen eine getrennte Geschäftslogik. Es sieht so aus als würde das alles in der GUI abgewickelt werden.
Ein paar von den besprochenen Änderungen (ungetestet):
Code: Alles auswählen
# coding: utf8
from __future__ import absolute_import, division, print_function
from tkinter import *
from tkinter.messagebox import showwarning
import pymysql
class Dbms(object):
def __init__(self, host, user, password, database):
self.dbms = pymysql.connect(
host=host, user=user, passwd=password, db=database
)
def _execute(self, sql, values=None):
cursor = self.dbms.cursor(pymysql.cursors.DictCursor)
cursor.execute(sql, values)
return cursor.fetchall()
def get_users(self):
return self._execute('SELECT * FROM schueler')
def get_open_items(self, user):
return self._execute(
'SELECT betrag, date, bezahlt, comment'
' FROM schulden'
' WHERE schueler_id = %s',
(user,)
)
def get_payment_statistics(self):
result = self._execute(
'SELECT bezahlt, COUNT(bezahlt) bezahlt_count, SUM(betrag) total'
' FROM schulden'
' GROUP BY bezahlt'
' ORDER BY bezahlt'
)
return [(row['bezahlt_count'], row['total']) for row in result]
class MainUI(object):
def __init__(self, master):
self.master = master
self.dbms = None
master.title('Klassenkasse')
master.resizable(width=0, height=0)
### Frames werden gesetzt ###
topframeleft = Frame(master, bd=2, relief=GROOVE)
topframeleft.grid(row=0, column=0)
topframeright = Frame(master, bd=2, relief=GROOVE)
topframeright.grid(row=0, column=1)
bottomframeleft = Frame(master, bd=2, relief=GROOVE)
bottomframeleft.grid(row=1, column=0, columnspan=2)
### TopFrameLeft ###
user_label = Label(topframeleft, text='Benutzername')
user_label.grid(row=0, column=0, pady=5, padx=5, sticky=E)
passwd_label = Label(topframeleft, text='Passwort')
passwd_label.grid(row=1, column=0, pady=5, padx=5, sticky=E)
self.entry_login = Entry(topframeleft)
self.entry_login.grid(row=0, column=1, pady=5, padx=5)
self.entry_passwd = Entry(topframeleft, show='*')
self.entry_passwd.grid(row=1, column=1, pady=5, padx=5)
button_login = Button(topframeleft, text='Login', command=self.login)
button_login.grid(row=0, column=2, rowspan=2, pady=5, padx=5)
### Topframeright ###
self.label_nochzuzahlen = Label(topframeright)
self.label_nochzuzahlen.grid(row=1, column=0)
self.label_bereitsgezahlt = Label(topframeright)
self.label_bereitsgezahlt.grid(row=2, column=0, columnspan=2)
self.label_summe = Label(topframeright)
self.label_summe.grid(row=3, column=0)
### BottomFrameLeft ###
button_insert = Button(
bottomframeleft, text='Eintragen', command=self.add_open_item
)
button_insert.grid(row=0, column=0, pady=5, padx=5)
button_send = Button(
bottomframeleft, text='Sende Erinnerung', command=self.send_reminder
)
button_send.grid(row=0, column=1, pady=5, padx=5)
button_bezahlt = Button(
bottomframeleft, text='Zahlung eingegangen', command=self.close_item
)
button_bezahlt.grid(row=0, column=2, pady=5, padx=5)
button_hilfe = Button(
bottomframeleft, text='Hilfe', command=self.show_help
)
button_hilfe.grid(row=0, column=3, pady=5, padx=5)
self.text_box = Text(master, bg='#8A8A8A', width=60, height=20, bd=2)
self.text_box.grid(
row=2, column=0, columnspan=2, padx=2, pady=2, sticky=S
)
self.listbox = Listbox(
master, bg='#4945F0', height=30, width=30, bd=2, relief=RAISED
)
self.listbox.grid(row=0, column=2, rowspan=3, padx=2, pady=2, sticky=N)
self.users = dict()
self.label_status = Label(
master, bg='#858585', width=95, bd=2, relief=RAISED
)
self.label_status.grid(column=0, row=3, columnspan=3)
def login(self):
try:
self.dbms = Dbms(
'192.168.2.100',
self.entry_login.get(),
self.entry_passwd.get(),
'bxt'
)
### Listet die User auf und speichert sie in die Rechte listbox ###
self.listbox.delete(0, END)
self.users = dict()
for user in self.dbms.get_users():
user_string = (
'{0["schueler_id"]}-{0["vorname"]}-{0["name"]}'.format(user)
)
self.listbox.insert(END, user_string)
self.users[user_string] = user
self.listbox.bind('<Double-Button-1>', self.on_user_selected)
(
(unpaid_count, unpaid_total), (paid_count, paid_total)
) = self.dbms.get_payment_statistics()
self.label_bereitsgezahlt['text'] = (
'Geschlossene Tickets : {0}'.format(paid_count)
)
self.label_nochzuzahlen['text'] = (
'Offene Tickets : {0}'.format(unpaid_count)
)
self.label_summe['text'] = (
'Aktuelle Summe : {0:.2f} €'.format(unpaid_total + paid_total)
)
except Exception as error:
showwarning('Fehler beim Einloggen', str(error))
status_text = 'Sie sind nicht angemeldet!'
else:
status_text = 'Sie sind angemeldet.'
self.label_status['text'] = status_text
def on_user_selected(self, _event=None):
user_key = self.listbox.get(self.listbox.curselection()[0])
self.label_status['text'] = 'Ein User wurde ausgewählt: {0}'.format(
user_key
)
user = self.users[user_key]
self.text_box.delete('1.0', END)
self.text_box.insert(
END,
'Vorname = {0["vorname"]}\nName = {0["name"]}\n\n'.format(user)
)
for row in self.dbms.get_open_items(user['schueler_id']):
self.text_box.insert(
END,
' Betrag = {0["betrag"]} €\n'
' Datum = {0["date"]}\n'
' Bezahlt = {1}\n'
' Kommentar = {0["comment"]}\n\n'.format(
row, 'J' if row['bezahlt'] else 'N'
)
)
def add_open_item(self):
pass
def send_reminder(self):
pass
def close_item(self):
pass
def show_help(self):
pass
def main():
root = Tk()
MainUI(root)
root.mainloop()
if __name__ == '__main__':
main()
