Path: csiph.com!fu-berlin.de!uni-berlin.de!not-for-mail
From: Stefan Schwarzer <sschwarzer@sschwarzer.net>
Newsgroups: de.comp.lang.python
Subject: =?utf-8?q?=5BPython-de=5D_Re=3A_Code_Style_Review?=
Date: Tue, 29 Nov 2022 18:18:38 +0100
Lines: 109
Message-ID: <25b8d1d4-d6e2-22ee-4bed-ec76227fa65d@sschwarzer.net>
References: <tm2jhn$1k5h8$1@news1.tnib.de>
Mime-Version: 1.0
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.24) Gecko/20100411 Thunderbird/2.0.0.24 Mnenhy/0.7.6.666
Content-Language: en-US, de-DE
In-Reply-To: <tm2jhn$1k5h8$1@news1.tnib.de>
Message-ID-Hash: Y6YXVAICX37E63AP654EAJ5NDBZNH4W5
Precedence: list
Archived-At: <https://mail.python.org/archives/list/python-de@python.org/message/Y6YXVAICX37E63AP654EAJ5NDBZNH4W5/>
Xref: csiph.com de.comp.lang.python:5873

Hallo Marc,

On 2022-11-28 16:19, Marc Haber wrote:
> ich wollte mich schon seit Jahren mal etwas ernsthafter mit python
> beschäftigen und hier ist mein Erstlingswerk signifikanter Länge.
> pylint hat nichts auszusetzen, an manchen Stellen habe ich pylint aber
> überstimmt.
> 
> Ist da irgendetwas drin, was nicht pythonesk genug formuliert ist?
> 
> Laufen tut das Programm jedenfalls.
> 
> Ich würde mich über Eure Kommentare freuen.

ich habe hier und da ein paar Anregungen. Wie immer gilt,
dass letztlich der Autor entscheidet, wie er/sie diese
Anregungen umsetzt. :-)

Erst mal zum Thema der Konfigurations-Speicherung, was in
dem Diskussions-Thread auch aufgegriffen wurde. Da in Python
Module und Klassen natürliche Singletons sind, wäre mein
Vorschlag, die Konfigurations-Daten in einer Klasse zu
speichern, also

```
class config:

     # Listener IP address and port of MQTT server.
     ip_address = None
     port = None

     # If `True`, print debugging output.
     debug = False

     ...
```

Du kannst einfach von überall aus dem Modul auf das
`config`-Objekt zugreifen und brauchst auch kein `global`.
Die Klasse wird nie instanziiert, sondern nur als Namespace
benutzt.

Der Ansatz hat gegenüber dem `config`-Dictionary auch den
Vorteil, dass du dokumentieren kannst, wofür die einzelnen
Werte gedacht sind. Ich halte es für gut, in `config` alle
Werte auf Default-Werte zu setzen, selbst wenn diese mangels
eines sinnvollen Defaults erst mal `None` sind. Das hat den
Vorteil, dass man sehen kann, was jemals gesetzt wird und
reduziert die Verwirrung, weil man nicht zwischen `None` und
"gar nicht gesetzt" unterscheiden muss. (Ich habe
tatsächlich mal Code gesehen, wo ein Dictionary zur
Datenübergabe verwendet wurde und die Abwesenheit eines Keys
eine andere Bedeutung hatte als der Key mit dem Wert `None`.
Das fand ich sehr verwirrend.)

Ich würde die Kommandozeilen-Optionen von den
`config`-Attributen getrennt halten, um nicht die Konzepte
Kommandozeilen-Parsen und Konfiguration zu vermischen. Das
ist aber vielleicht ein Detail.

Ich empfehle, falls möglich die `config`-Klasse erst mit
allen gewünschten Werten zu füllen oder zu überschreiben
und dann nur noch lesend darauf zuzugreifen.

Zu den Debug-Ausgaben: Da kannst du den Code etwas
vereinfachen, indem du die Abfrage des Debug-Flags in eine
Funktion verschiebst, die du aufrufst. Also statt

```
if config.debug:
     print(...)

if config.debug:
     print(...)

if config.debug:
     print(...)
```

eher

```
def debug_print(*args, **kwargs):
     if config.debug:
         print(*args, flush=True, **kwargs)

debug_print(...)

debug_print(...)

debug_print(...)
```

(Bei der Gelegenheit kannst du noch wie oben ein
`flush=True` spendieren, was beim Debuggen von nebenläufigem
Code für eine übersichtlichere Ausgabe sorgen kann.)

Für `element` würde ich statt Dictionarys `namedtuple`s oder
Instanzen von `SimpleNamespace` verwenden. Allgemein sehen
Zugriffe auf Dictionarys aus meiner Sicht immer recht
"noisy" aus.

Es gibt vermutlich noch mehr zu verbessern, aber das kann
ich jetzt nicht so leicht überblicken. Oft geht solches
Aufräumen iterativ besser, also bewerten, verbessern, neu
bewerten, weiter verbessern usw.

Viele Grüße
Stefan