Das deutsche Python-Forum

Hallo,
ich bräuchte einige Ratschläge wie ich meinen Code verbessern kann, der rechnet mir nämlich die Punkte nicht von jeder Runde zusammen, sonder nur von der aktuellen: ich bekomme als Lösung nur die Punktezahl von der letzten Runde als Output.

def calc_points(words):

letter_points = []


words_letters = []


common_elements = []


total_points = []


points = 0


for key in POINTS.keys():
letter_points.append(key)


for n in range(0, len(words)):
letters = words[n]
words_letters.append(letters)


for letter in words_letters:
if letter in letter_points:
common_elements.append(letter)
else:
continue


for i in common_elements:
total_points.append(POINTS)


for number in total_points:
points += points+int(number)

return points

@eycey71: Die Leerzeilensetzung ist ziemlich grosszügig. Was soll das bedeuten?

Man definiert Variablen nicht alle am Anfang sondern erst dann wenn man sie braucht.

Für `letter_points` braucht man keine Schleife, da hätte man einfach ``letter_points = list(POINTS)`` schreiben können. Allerdings braucht man diese Liste überhaupt gar nicht, denn man kann auch direkt das `POINTS`-Wörterbuch verwenden um mit ``in`` zu prüfen ob ein Schlüssel enthalten ist. Und das ist sogar effizienter als das mit einer Liste zu machen.

`words_letters` braucht man auch überhaupt nicht, denn das ist nur eine Kopie von `words`, es wird aber gar keine Kopie benötigt, man kann da wo `words_letters` verwendet wird auch einfach das Original `words` verwenden.

Das ``else: continue`` ist sinnlos, das kann weg.

Bei der Schleife die `common_elements` ist der Name `letter` für *ein* Wort irgendwie falsch. Nenn das doch einfach `word`. Und falls `POINTS` tatsächlich Buchstaben als Schlüssel hat, dann haben wir hier einen Fehler in dem Programm und dann würde diese Funktion immer 0 zurückgeben (es sei denn es gibt Wörter die nur aus einem Buchstaben bestehen). Dieser Fehler hätte an der Wortwahl dann aber auch auffallen müssen, denn ``for letter in words:`` klingt ja schon falsch.

Die Schleife ist so simpel das man sie als „list comprehension“ schreiben kann: ``common_elements = [word for word in words if word in POINTS]``

`i` ist ein sehr schlechter Name für Worte. Und `total_points` in einer zusätzlichen Schleife zu erstellen ist auch reichlich umständlich. Die vorhergehende Schleife/„list comprehension“ muss dafür ja nur ganz minimal geändert werden um diesen Job mit zu erledigen, es fehlt ja nur der Zugriff per `word` auf `POINTS`: ``total_points = [POINTS[word] for word in words if word in POINTS]``

Das hier ist wohl auch falsch: ``points += points+int(number)``. Und das hätte ja eigentlich auch auffallen müssen das die Punktzahl am Ende viel zu hoch ist.

Der `int()`-Aufruf hat da auch nichts zu suchen. Die Punkte sollten in `POINTS` bereits Zahlen sein.

Letztlich bleibt von der Funktion nur noch diese eine Zeile übrig:

Da es sich laut Überschrift um Scrabble handelt, ist bei words vielleicht auch einfach ein s zu viel: Von welchen Runden sprichst Du?
Liegt der Fehler in den Teil des Programms, den du nicht zeigst?