@pyron: Namen sollte nach Möglichkeit keine Typen enthalten. Also zum Beispiel `get_klassenliste()` -- wenn man da den Rückgabetyp mal ändert, müsste man den Namen ändern oder man hätte einen irreführenden Namen. `get_klassen()` ist ausreichend. Die "Methode" hat IMHO auf `Lagerplatz` nichts verloren. Es ist ja nicht einmal eine Methode sondern einfach nur eine Funktion. Wenn es weder die Klasse noch ein übergebenes Exemplar davon benötigt, dann sollte man es als Funktion schreiben, oder zumindest mit `staticmethod()` klar machen, dass es keine Methode ist, sondern eine Funktion die thematisch mit der Klasse zu tun hat.
Die `Lagerplatz.__str__()` ist auch deutlich zu kompliziert. Das ist ein Einzeiler:
Code: Alles auswählen
def __str__(self):
return str([self.nummer, self.empty, self.klasse, self.variante])
Einen Docstring würde ich solchen "magischen" Methoden nur verpassen, wenn da Informationen drin stehen, die man sonst nicht hätte. Was die `__str__()` Grundsätzlich tut, sollte man als bekannt voraussetzen dürfen.
Abkürzungen sollte man vermeiden, insbesondere wenn es sich um welche handelt, die nicht allgemein bekannt sind. `sucheLP()` sollte `suche_lagerplatz()` heissen. Es sei denn die Methode sucht tatsächlich Langspielplatten. Ebenso sind einbuchstabige Namen ausserhalb von Mathematik-Problemen und Zählvariablen in Schleifen unschön. Sehr kurze lokale Namen in "list comprehensions" und Generator-Ausdrücken sind auch noch okay, weil da der Einsatzbereich eng genug gefasst ist, dass man nicht den Überblick verliert. Ebenfalls nicht schön sind generische Namen wie `list` oder `liste` wenn man viel erklärendere Namen vergeben könnte. Die `list` in `sucheLP()` würde zum Beispiel besser `lagerplaetze` heissen. Dann weiss man auf den ersten Blick was darin gespeichert ist.
In `sucheLP()` wird auf das globale `Posten` zugegriffen: Das ist total unsauber und macht Quelltext schwer überschaubar. Und in diesem Fall würde ich sogar sagen, dass das ein Programmfehler ist. Werte die innerhalb einer Methode benutzt werden, sollten vom Objekt oder über die Argumentliste kommen. Auf Modulebene kann man auf Konstanten zugreifen, aber nicht irgendwelche Veränderungen an modulglobalen Objekten vornehmen. Die Schleifen scheinen mir auch ziemlich umständlich. Filtern der Liste nach in Frage kommenden Lagerplätzen und davon dann das Minimum von Klasse/Variante zu suchen erscheint mir irgendwie viel einfacher!? Ungetestet:
Code: Alles auswählen
def suche_lagerplatz(self, lagerplaetze):
#
# Belegte und zu kleine Lagerplätze ausfiltern.
#
kandidaten = (
lp for lp in lagerplaetze
if lp.empty
and lp.klasse >= self.klasse
and lp.variante >= self.variante
)
#
# Kleinsten möglichen Lagerplatz zurück geben.
#
try:
return min(kandidaten, key=lambda lp: (lp.klasse, lp.variante))
except ValueError:
raise ValueError('Keinen Lagerplatz gefunden.')
Hier wird auch nicht `None` zurück gegeben, sondern ein `ValueError` ausgelöst, wenn kein passender Lagerplatz gefunden wird. Ich gehe ausserdem davon aus, dass `empty` ein Wahrheitswert sein sollte und keine Zeichenkette mit einem Text, der einen Wahrheitswert darstellt.
`sucheLP()` und `findeLP()` sind mir in der Unterscheidung des Namens zu subtil. Da sollte man bessere Namen finden, so dass man nicht raten muss, was denn der Unterschied zwischen den beiden ist.
Vom Entwurf her sind IMHO einige Methoden auf den falschen Objekten. Die Klassen und Varianten aus einer Liste zu suchen, sollte nicht Aufgabe eine `Lagerplatz`\es sein, genau so wenig wie das suchen eines passenden `Lagerplatz`\es in einer Liste zum `Artikelposten` gehört. Beides wären Aufgaben einer Klasse die `Lagerplatz`-Objekte enthält und `Artikelposten` dort ablegen und heraus holen kann. Ausgaben an den Benutzer gehören in solche Programmlogik-Klassen auch nicht hinein.
Das Hauptprogramm sollte in einer Funktion verschwinden. Dann können solche Fehler wie der Zugriff auf das globale `Posten` aus einer Methode heraus auch gar nicht erst passieren.
Die Liste im Hauptprogramm wird wieder reichlich umständlich erstellt. Die Namen `Bamler10` bis `Bamler20` sind unnötig.
Edit: Vielleicht noch als Ergänzung: Wenn als "natürliche" Ordnung von Lagerplätzen die Klasse und Variante in Frage kommen, könnte man `Lagerplatz`-Objekte auch auf grösser/kleiner/gleich vergleichbar machen und sich damit die `key`-Funktion bei `min()` sparen. Der Test ob ein Posten in einen Lagerplatz passt, könnte man auch auf den Lagerplatz verlegen. Wenn man dass dann noch auf eine Container-Klasse verschiebt, die Lagerplätze verwaltet, könnte das letztendlich so aussehen:
Code: Alles auswählen
def _suche_lagerplatz(self, posten):
kandidaten = (lp for lp in self if lp.could_contain(posten))
try:
return min(kandidaten)
except ValueError:
raise ValueError('Keinen Lagerplatz gefunden.')