Script Optimierung: getopts, argumente, fehlerhandling, etc.

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
derdon
User
Beiträge: 1316
Registriert: Freitag 24. Oktober 2008, 14:32

So, hab versucht, es etwas auszubessern: http://paste.pocoo.org/show/487048/. Wenn die Angaben von der Sprache und Ort nicht zusammen passen, gibt es eine Fehlermeldung wie hier (dafür sorgen die Zeilen 40-43):

Code: Alles auswählen

./tmp.py 1234 -l de -t US
usage: tmp.py [-h] [-l LANGUAGE] [-t TERRITORY] ip
tmp.py: error: unknown locale 'de_US'
Wird keine Angabe zu Sprache / Ort gemacht, wird auf die lokalen Einstellugen der Shell zurückgegriffen. Ich konnte es nicht komplett testen, weil ich das geoip Paket nicht installiert habe. Normalerweise lasse ich bei meiner parse_options gleich die `args` zurückgeben, aber wegen Zeile 43 wird ja ein ArgumentParser-Objekt benötigt, daher lasse ich ein parser-objekt zurückgeben.

Edit: Noch eine Anmerkung: Dass man bei der Funktion set_default_locale ein anderes Modul (oder genau genommen irgendein Objekt) übergeben kann als das locale-Modul aus der stdlib, liegt hauptsächlich daran, dass sie sich so leichter testen lässt (Stichwort testability). Ein netter Nebeneffekt ist außerdem, dass diese Funktion so schneller ist, weil auf lokale Namen schneller zugegriffen werden kann als auf globale Namen. Das ist aber nur bei sehr vielen Funktionsaufrufen relevant. Man sollte also noch dokumentieren, dass "der normale Benutzer" diese Funktion ohne Parameter aufruft; das habe ich vergessen.
Benutzeravatar
think
User
Beiträge: 37
Registriert: Donnerstag 22. Februar 2007, 10:02
Wohnort: Schweiz

Wenn man noch ein "Import GeoIP" anfügt funktioniert es perfekt. :-)

Herzlichen Dank jedenfalls an alle Helfer!
derdon
User
Beiträge: 1316
Registriert: Freitag 24. Oktober 2008, 14:32

Ja, das kommt davon, wenn man zwischendurch einiges auskommentiert, um es auch ohne GeoIP testen zu können :lol:
lunar

@think: Bis auf die fehlende Fehlerbehandlung, die derdon schon erwähnt hat, ist das Skript soweit gut.

@derdon: "parse_option()" ist irreführend, den die Argumente werden in dieser Funktion überhaupt nicht betrachtet. Entweder gibt die Funktion die geparsten Argumente zusammen mit dem Parser zurück, oder man nennt die Funktion "create_parser()" oder ähnlich.

"set_default_locale()" finde ich auch unschön. Die Funktion vermischt zwei grundverschiedene Dinge, nämlich die Initialisierung der Sprachumgebung sowie das Herausfinden der Sprache. Zudem ist sie irreführend benannt, denn ihr eigentlicher Zweck ist das Herausfinden der Sprache und des Landes, nicht das Initialisieren der Sprachumgebung.

"locale" zu übergeben, um die "testability" zu verbessern, ist meines Erachtens etwas übertrieben, tut die Funktion doch nicht mehr, als die "locale"-API zu nutzen. Darauf kann man sich auch ohne testen verlassen, und mithin zugunsten der Lesbarkeit auf die umständliche Übergabe verzichten. Ich halte es zudem für problematisch, Quelltext zu schreiben, der nur und ausschließlich zur Unterstützung der Tests dient. So testet man defacto Quelltext, der produktiv niemals ausgeführt wird, und müllt den Quelltext mit Logik zu, die für den eigentlichen Zweck des Programms vollkommen irrelevant ist. Ich halte es für sinnvoller, Tests und Quelltext strikt voneinander zu trennen, und nach Möglichkeit Quelltext zu testen, der in dieser Form auch wirklich im Produktivsystem ausgeführt wird. Testen kann man die Funktion ja trotzdem noch über Monkey-Patching in den entsprechenden Tests. Das ist in Python problemlos möglich und wird von jeder vernünftigen Testbibliothek unterstützt.

Wenn man die Funktion so implementieren möchte, dann besser als "get_default_language_and_territory/()" mit "locale.getdefaultlocale()", um den Seiteneffekt loszuwerden und den Zweck der Funktion zu verdeutlichen.

Ich würde die Funktion aber reduzieren, zu einem "parse_language_code(language_code)", aufgerufen in "main()" als "language, territory = parse_language_code(locale.getlocale()[0])" in "main()". Vorher ist natürlich noch ".setlocale()" nötig. Da diese Funktion den gesamten Prozess betrifft, gehört sie imho in die Eintrittsfunktion, und zwar zum frühestmöglichen Zeitpunkt, noch vor dem Parsen der Argumente.

So sind die Seiteneffekte nach Möglichkeit vom Rest des Programms getrennt, jede Funktion hat eine klare Aufgabe und ist unabhängig zu testen.
Benutzeravatar
think
User
Beiträge: 37
Registriert: Donnerstag 22. Februar 2007, 10:02
Wohnort: Schweiz

lunar hat geschrieben:@think: Bis auf die fehlende Fehlerbehandlung, die derdon schon erwähnt hat, ist das Skript soweit gut.
Bis hierhin bin ich gut mitgekommen. *g*
-> Fehlerbehandlung ist klar, de_US ist selbstverständlich ungültig, wird aber momentan nicht abgefangen.

Den Rest muss ich mir mal in einer ruhigen Minute durchlesen. Ich denke zwar - mehr oder weniger - verstanden zu haben, was du meinst, bin mir aber nicht überall sicher.

Ich werde das Script anhand deiner Gedanken versuchen zu ändern und dann hier posten. :-)
Antworten