Das deutsche Python-Forum

Joa, jetzt bin ich auch mal dran, wa?

Der WikiTranslator ist ein kleines Python-Tool, welches automatisch bestimmte Teile von Artikeln im offiziellen TF Wiki übersetzt. Die Zielgruppe dafür ist zwar ziemlich beschränkt, aber würde mir auch gerne mal so'n bisschen Rat zum Coding-Stil, Aufbau und sonstigem holen, ich will ja auch mal besser werden.

Das Tool übersetzt allerdings nicht Wort für Wort und nutzt schon gar keine semantischen Analysen. Stattdessen werden einzelne Informationen (Gegenstandsname, Level usw.) aus dem englischen Text extrahiert und in vorgefertigte Sätze eingefügt. Ansonsten kann u.A. auf die MediaWiki-API zugegriffen werden, um Wikipedia- und Wikilinks zu übersetzen.
Das Ganze gibt's auch in Kombination mit einer GUI, mit der man alle Seiten aus einer Kategorie importieren kann, Texte speichern und öffnen kann, usw.
Es lassen sich auch einzelne Übersetzungsfunktionen auswählen und in Presets speichern.

Der gesamte Quelltext liegt hier: https://github.com/TidB/WikiTranslator
Dabei ist sDE.py eine einfache Ansammlung von Konstanten, die vordefinierte Textbausteine und Dictionarys enthalten. Der Name ist einfach "s" + der ISO-Code einer Sprache, damit man später mehr davon hinzufügen kann.
core.py enthält die Klasse Wikitext, die alle Funktionen zum Übersetzen dieses Textes bereitstellt.
gui.py enthält völlig überraschend das GUI.

Das Ganze wurde in Python 3 geschrieben (ja, wirklich) und nutzt zweimal eval() (bitte nicht schlagen)

Nun ja, ich bin gespannt auf euer Feedback ;)

@Schorlem: Also ich zähle da mehr als zwei `eval()` und keines davon ist notwendig. Es gibt `getattr()`. Beispiel: Und für Übersetzungen bietet sich das `gettext`-Modul an, das ist schliesslich speziell für diesen Zweck gemacht.

Ach stimmt, ich hatte die ganzen "Kaskaden" vergessen^^

Und das mit gettext schaue ich mir auch mal an, mal sehen, was da rauskommt. Geht der Rest denn so grob in Ordnung oder sind da noch Unstimmigkeiten?
Vielen Dank schonmal für die Hinweise ;)

@Schorlem: Das sind ja ziemlich viele Zeilen Quelltext. Also mal ein paar Anmerkungen, ohne Anspruch auf Vollständigkeit:

Die Namensschreibweisen halten sich nicht alle an den Style Guide for Python Code.

`gui` und `core` sind als Modulnamen nicht gerade eindeutig, das sollte also in einem Package verschwinden was einen passenden Namen hat.

Statt mit `urllib` würde ich mit der externen `requests`-Bibliothek arbeiten. Das ist eine wesentlich schönere API.

`str()` würde ich nicht zum Dekodieren verwenden. Damit macht man es unnötig schwer den Code unter Python 2.x lauffähig zu machen. Die `decode()`-Methode funktioniert bei beiden Python-Major-Versionen.

Das ”bereinigen” von Funktionsargumenten wie in `import_category()` würde ich *vor* dem Aufruf machen.

Die Programmlogik sollte keine Fehlerausgaben für den Benutzer enthalten. Wenn die Kategorie nicht gefunden werden konnte sollte man ausserdem keinen speziellen Fehler-Rückgabewert verwenden sondern eine Ausnahme auslösen. Bei dem `None` gibt's bei Dir in der GUI dann sowieso eine Ausnahme weil Du darauf gar nicht prüfst.

Für das Laden von und Parsen von XML über eine URL hast Du x-mal die fast gleichen Quelltextzeilen im Programm stehen.

`i` speziell als Laufvariable in einer Schleife solle man nicht an andere Typen als ganze Zahlen binden. Das verwirrt sonst jeden erfahrenen Programmierer. In der `import_category()` ist das sowieso ein schlechter Name weil er nichts über die Bedeutung des Wertes verrät.

Konkrete Datentypen sollten nicht in Namen stehen. Wenn man den Datentyp mal ändert, muss man entweder überall auch den Namen ändern, oder man hat einen irrefüherenden Namen im Programm. Für Containter-Objekte bietet sich in der Regel die Mehrzahlform von dem Namen an, dem man einem einzelnen Element geben würde. Also zum Beispiel `parrots` statt `parrot_list`.

Um ``continue`` mache ich einen grossen Bogen. Das ist unüberichtlich weil es ein Sprung aus einem Schleifenkörper an den Anfang ist, der nicht aus der Einrückung ersichtlich ist. Der Sprung schränkt einen ein, weil man danach nichts mehr einfügen kann was garantiert am Ende jedes Schleifendurchlaufs ausgeführt wird, und es kann das herausziehen von Code aus zu tief verschachtelten Strukturen erschweren oder gar verhindern.

Bei der Klasse ist `iso` ein schlechter Name weil der nicht wirklich vermittelt das es sich um die Angabe einer Sprache handelt.

Einige der Methoden sind keine (und haben auch ganz furchtbare Namen) sondern Funktionen. Die gehören also entweder nicht in die Klasse, oder man sollte sie als `@staticmethod` implementieren.

Auch die Klasse enthält wieder Benutzerinteraktion die nicht in die Programmlogik gehört.

Nackte ``except``\s sind böse. Gerade in der `Wikitext.__init__()` machst Du Dir so die Fehlersuche unmöglich.

`__str__()` muss die Zeichenkettendarstellung *zurück*geben und nicht *aus*geben (und implizit `None` zurückgeben).

Und an der Stelle höre ich jetzt erst einmal auf.

BlackJack hat geschrieben:`gui` und `core` sind als Modulnamen nicht gerade eindeutig, das sollte also in einem Package verschwinden was einen passenden Namen hat.

Findest du grundsätzlich, dass ``gui`` und ``core`` schlechte Modulnamen sind oder ist das speziell im Kontext des hier vorgestellten Projektes gemeint?

@Schorlem: Deine ``core.py`` sollte IMHO kein Wissen davon benötigen, welche ihrer Methoden in anderen Zusammenhängen genutzt werden. Konkret beziehe ich mich auf ``core.GUI_METHODS``. Man macht die niedriger angesiedelte Ebene ("core") damit zwar nicht direkt abhängig von der höher angesiedelten ("gui") - was man nämlich tunlichst vermeiden sollte - aber es ist dennoch ein unglücklich gewähltes Design. Du kannst die Methodennamen ja genau so gut in der Logik der GUI ermitteln.

@snafu: Die Namen sind nicht generell schlecht, aber auf der Ebene wo sie definiert sind, sind sie viel zu allgemein. Die stehen ja in ”Konkurrenz” mit den Modulen aus der Standardbibliothek und Drittbibliotheken. Wenn man jetzt eine Bibliothek importiert die ihrerseits ein `core`-Modul besitzt und das importieren möchte, kann es passieren dass nicht *deren* sondern das eigene `core`-Modul importiert wird. Und das ist blöd, weil diese Bibliothek sicher nichts mit dem eigenen `core`-Modul anfangen kann.

Darum würde ich so etwas in ein Package stecken und explizit aus dem Package importieren, also zum Beispiel ``from wikitranslator import core``. Die Gefahr das eine andere Bibliothek aus versehen `wikitranslator.core` importiert ist wesentlich geringer.

Grundsätzlich habe ich mir angewöhnt alles was mehr als ein Modul hat in ein Package zu stecken. Ist einfach sauberer/sicherer.

BlackJack hat geschrieben:Grundsätzlich habe ich mir angewöhnt alles was mehr als ein Modul hat in ein Package zu stecken. Ist einfach sauberer/sicherer.

Sehe ich ganz genau so. Wobei ich relative Importangaben im eigenen Package (z.B. ``from . import core``) bevorzuge, um nicht jedes Mal den Package-Namen mitschreiben zu müssen und um Problemen bei späteren Umbenennungen oder Änderungen an der Package-Hierarchie etwas vorbeugen zu können.