OK, der Reihe nach, erst @__deets__:
Ein *-Select, von dem dann nachtraeglich das erste Element entfernt wird? Warum nicht einfach gleich die explizite Liste der Spalten angeben?
Diese Kritik kam ja auch von Sirius. Es sind insgesamt rund 50 Spalten, die hier geladen werden sollen – fast der gesamte Inhalt der Datenbank. Jede einzelne im Select zu nennen, ist etwas...unhandlich. Daher dachte ich, ein * mit Abzug der ersten Spalte wäre zielführender.
Auch die bare excepts wurden mehrfach genannt. Danke für den Hinweis, werde hier stattdessen if/else verwenden, außer es gibt einen guten Grund.
Danke für den Hinweis zu den formatierten Datumswerten. Ich hatte schon vor dem Review vor, hier mal einzugreifen, weil ständig irgendwas hin- und herformatiert wird und so einfach nur Chaos erzeugt. Der Salat ist mit dem Programm angewachsen und irgendwann war es so eingerbeitet, dass es ohne großen Aufwand nicht mehr rauszunehmen war. Garstig.
Jetzt @Sirius:
Zeile 16: ein Frame sollte sich nicht selbst positionieren
OK. Das direkte Erben von der Frame-Klasse war ein Experiment, weil ich damit vorher noch nicht gearbeitet hatte. Mir ist nicht klar, wo ich den den Frame sonst positionieren sollte, außer ich breche die Struktur auf und erzeuge ihn „normal“, also nicht als Erbe der Superklasse.
Zeil 30ff: wenn die Lokale anders eingestellt ist, dann wohl aus gutem Grund
Auf meinem PC müsste ich auch nix ändern. Meine Frau benutzt einen Mac, und dort sind die TKInter Schaltflächen (z.B. Messagebox) ohne ein Ändern der Lokale standardmäßig Englisch. Obwohl im Prinzip kein Problem, entsteht so Sprachmischmasch und das sieht das nat. unschön aus.
Zeile 157ff: im Prinzip das selbe wie für load_db_loc schon beschrieben.
Mir ist es absolut recht, mehr Klassen einzufügen. Ich gliedere lieber einmal zu viel als zu wenig, aber ich hab echt Skrupel, eine Klasse aus zwei Funktionen (die wir z.B. bei den hier erwähnten Passwort Dialogen haben) zu schreiben. Ist das nicht etwas wenig?
Zeile 223: fehleranfällig, falls einer der Strings nicht die erwartete Länge hat.
Ich würde hier dann tatsächlich mit einem try/except arbeiten. Hat einer der Strings nicht die erforderliche Länge (z.B. 0, weil der Nutzer ein Dropdown Menü leer gelassen hat), wird ein ValueError geworfen. Das ist a) relativ denkbar und b) relativ präzise vorhersagbar und handhabbar, deswegen halte ich ein except ValueError für vertretbar. Oder?
Zeile 483: x ist ein schlechter Name für eine Datum [u.a.]
Ja. Verzeih mir, Meister, denn ich war unkreativ – werde es ausbessern.
Zeile 485: Du solltest Datums-Objekte vergleichen und nicht Strings, also start_from und end_from in datetime umwandeln, statt umgekehrt.
Siehe oben. Die Datumsgeschichten sind ziemlich außer Kontrolle und müssen dringend neu geschrieben werden.
Zeile 489: z ist ein schlechter Name für einen Job. Was soll dieses Inkrement überhaupt?
Das Inkrement fischt alle None-Werte aus der Liste raus. Dann weiß man, wieviele Viertelstunden gearbeitet wurden, kann das durch vier Teilen und bekommt die Stundenanzahl.
Zeile 496: das macht list(range()), sowieso überflüssig, weil das der Default von plot ist.
Das verstehe ich nicht ganz...
Zeile 498: Schreibfehler.
Zeile 516: was soll diese Liste?
Falls du das suptitle() meinst: ne, das soll eine Überschrift werden. Die Liste ist überflüssig, ein Relikt, das ich vergessenhabe zu entfernen. Ist raus.
Dann das Thema Datenbank. Da scheint ja das meiste im Argen zu liegen...daher zunächst mal allgemein: wie könnte man das Design verbessern? Die zahlreichen Spaltennamen habe ich ja schon angesprochen; ich nehme an, dass auf die suboptimale Konstruktion dieser Spaltennamen auch die Kritik an der Parametrisierung abzielt.
Ich muss in der Datenbank für jeden Tag verschiedene Tätigkeiten speichern, die zu unterschiedlichen Zeiten ausgeführt wurden. Dazwischen können „Pausen“ liegen, in denen nicht dokumentationspflichtige Arbeiten erledigt werden. Das zeitliche Raster reicht von 7:00 morgens bis 21:00 abends und muss leider im Abstand von 15 Minuten angelegt werden, um die Kleinteiligkeit korrekt abbilden zu können. Egal, wie ich es gedreht und gewendet habe: das Design der Datenbank war bei mit immer beschissen.
Viele der von euch erwähnten Fehler sind auf Workarounds zurückzuführen, die aufgrund dieses Designs nötig werden.
Konkret noch eine Frage:
Zeile 799: Cursors sind etwas kurzlebiges, sollte nicht an ein Attribut gebunden werden.
Naja, ich brauche den Cursor ja immer wieder für Tätigkeiten in der Datenbank. Demnach müsste ich in fast jeder Methode dieser Klasse erstmal einen neuen Cursor erzeugen – ist das nicht doppelter Code?
Insgesamt schlage ich vor, dass ich in GitHub den Code von Grund auf neu schreibe und einstelle. Bevor ich das mache: erstmal einen soliden Plan machen, wie das nun aussehen soll. Wieviele Module/Packages? Wo muss die bisherige Codestruktur besonders in weitere Klassen unterteilt werden, wo muss der ganze "Algorithmus" überarbeitet werden (z.B. Datenbank)? Usw.