@docmcfly: Fangen wir mal mit dem Unwichtigsten an: Namenskonventionen.
Da gibt's
PEP 8 -- Style Guide for Python Code. Und `ServiceEntity` kling auch sehr nach Java. Da wird ja dazu geneigt eigentlich überflüssiges an den Namen anzuhängen wie `Entity`, `Manager`, und so weiter. Einfach nur `Service` würde es doch auch tun. In Java ist das ja meistens weil man noch ein paar andere Klassen oder Interfaces drumherum hat, die sonst alle gleich heissen müssten. Also zum Beispiel das man ein `Service`-Interface hat und die konkrete Klasse dann halt anders nennen *muss*.
Ich bin mir nicht sicher ob Du bei Python Klassenattribute verstanden hast. Das was auf Ebene des Klassenkörpers an Namen gebunden wird, wäre in Java ``static``. Und Du nutzt den Umstand für Defaultwerte wo im Laufe der Zeit dann (eventuell) zusätzlich noch Attribute mit dem gleichen Namen auf dem Exemplar eingeführt werden. Du hast also diese Namen doppelt und je nach dem ob er auf dem Exemplar existiert oder nicht, wird beim Lesezugriff auf das Klassenattribut oder das Exemplar-Attribut zugegriffen. Das kann in einigen wenigen Fällen sinnvoll sein, ist aber etwas unsauber. In der Regel würde man auf Klassen nur Konstanten ablegen und die auch entsprechend benennen. Wo es zum Beispiel absolut keinen Sinn macht ist `ServiceEntity._directory`. Das Attribut hast Du einmal auf der Klasse und dann noch einmal auf jedem Exemplar. Das auf der Klasse ist *immer* an `None` gebunden und wird *nie* benutzt. Das kann man sich also auch sparen.
`os.path` muss man nicht importieren, dass `path`-Attribut wird schon durch das importieren von `os` an das Modulobjekt gebunden.
Es ist zwar sowieso nicht so ganz plattformunabhängig, aber wenn man schon `os.path.join()` verwendet, dann sollte man bei den Argumenten nicht schon plattformspezifische Pfadtrenner drin haben.
Namen sollten nicht zu kurz und zu kryptisch sein. Man sollte keine Abkürzungen verwenden, solange die nicht allgemein bekannt sind. `_l` ist zum Beispiel ein ganz schlechter Name. `log` oder `_log` ist nun nicht so viel länger, aber man weiss was das ist, wenn man es liest.
Die `ServiceEntity.__init__()` macht auf mich ein wenig den Eindruck als wenn dort zwei Methoden in eine geschrieben wurden. Das würde ich trennen, oder das `options`-Argument durch mehrere Argumente ersetzen und dafür dann ein wenig von dem Code aus der `__init__()` zum Aufrufer verlagern. Das kann man ja so ein bisschen als UI-Code ansehen, der in die Programmlogik gerutscht ist.
`set_directory()` wird in der `__init__()` inkonsistent verwendet und nirgends scheint der Rückgabewert verwendet zu werden!?
`os.path.exists()` ist eine Funktion die man sich in sehr vielen Fällen sparen kann. Wenn man danach die Datei öffnet, kann es trotzdem zu einer Ausnahme kommen, weil zwischen den beiden Funktionen die Datei gelöscht worden sein kann, also muss man wenn es robust sein soll, sowieso Ausnahmebehandlung beim `open()` machen. Da wäre dann der Fall das die Datei nicht existiert auch mit abgedeckt.
Dateien sollte man mit der ``with``-Anweisung öffnen um sicher zu stellen, dass sie auch in jedem Fall wieder geschlossen wird.
In `fobj` ist das `obj` etwas redundant, denn es ist ein Name und in Python ist *alles* was man an einen Namen binden kann, ein Objekt. `obj` ist also keine Zusatzinformation für den Leser.
Bei `read_pid()` finde ich es etwas inkonsistent, dass eine PID-Datei mit falschem Inhalt eine Ausnahme auslöst, eine mit richtigem Inhalt wo es den Prozess aber nicht (mehr) gibt, nicht von *keiner* PID-Datei unterschieden werden kann.
Statt ``not x is y`` sollte man ``x is not y`` schreiben. Das ist nur ein Operator und es ist lesbarer. Bei ``if not self._notifier is None or not pid is None:`` musste ich zumindest mehr nachdenken als mir lieb war.
(An der Stelle könnte man eventuell auch ausnutzen was Python in diesem Kontext als wahr und falsch ansieht und einfach ``if self._notifier and pid:`` schreiben, unter der Annahme, dass es keine PID mit dem Wert 0 geben darf.
Der Name `msg` wird dort übrigens an einen Wert gebunden, dann aber nicht verwendet.
In der Programmlogik sollte man niemals `exit()` aufrufen. Damit kann man diesen Code nicht einfach wiederverwenden. Vielleicht möchte man ja eine Aufrufebene höher gerne etwas weniger drastisch auf bestimmte Bedingungen reagieren als den *gesamten* Prozess zu beenden. Stell Dir mal vor Du willst das Programm auf mehrere unabhängig überwachte Verzeichnisse erweitern und dazu werden dann mehrere `Service`-Objekte erstellt. Und wenn es bei einem davon Probleme beim Starten gibt, wird gleich alles beendet, obwohl andere problemlos gestartet sind und arbeiten.
Die `make_dir()` ist viel zu aufwändig und Pfad oder `None` sind IMHO die falschen Rückgabewerte. Es sollte der Pfad zurückgegeben werden oder eine Ausnahme ausgelöst werden, wenn es den nicht gibt. Spezielle Fehlerwerte sind ein Relikt aus Sprachen, bei denen es keine Ausnahmen gibt. Genau so etwas wollte man mit Ausnahmen los werden.
`EventHandler.__init__()` ruft nicht die `__init__()` der Basisklasse auf.
`make_dir()`, `archive_archive()`, und `create_new_filename()` müssten nicht zwingend Methoden sein. Nur so als Anmerkung für jemanden der von Java kommt.
In der `__init__.py` erstellst Du ein `ServiceEntity`-Exemplar, dass nirgends verwendet wird!?
Und `LocalHistoryWindow` ist syntaktisch nicht korrekt. Ein Versuch das Ganze zu starten endet in einem `IndentationError`:
Code: Alles auswählen
$ python ./bin/local-history
Traceback (most recent call last):
File "./bin/local-history", line 32, in <module>
import local_history
File "/home/marc/tmp/local-history/local_history/__init__.py", line 14, in <module>
from local_history import LocalHistoryWindow
File "/home/marc/tmp/local-history/local_history/LocalHistoryWindow.py", line 30
def finish_initializing(self, builder): # pylint: disable=E1002
^
IndentationError: expected an indented block
und das auch noch mit dem potenten Version-Syztem git verbunden.