@naomii2608: Ich hatte es ja schon mal geschrieben: Dateien die man öffnet sollte man auch wieder schliessen. Am besten verwendet man wo es möglich ist die ``with``-Anweisung um das auch in jedem Fall sicherzustellen.
`grades_and_names` beschreibt keine Tätigkeit. Das wäre ein passender Name für das Ergebnis der Funktion den man aber nicht mehr verwenden kann, weil die Funktion ja schon so heisst. Wobei es umgekehrt richtiger wäre, denn die Daten enthalten ja erst den Namen und dann die Noten. Beim Argument von `format_data()` wird es dann ja richtig herum geschrieben.
Grunddatentypen gehören nicht in Namen. Die `grade_list` ist dann einfach nur `grades` oder eben `names_and_grades`.
Der `readlines()`-Aufruf ist überflüssig. Man kann über das Dateiobjekt itereren und bekommt auch so die Zeilen, ohne dabei gleich die gesamte Datei in den Speicher zu laden.
`i` ist ein verdammt schlechter Name für eine Zeile. In einer Schleife als Laufvariable nochmal schlechter weil *jeder* hier eine ganze Zahl hinter dem Namen vermutet.
In der Schleife wird dann 3× die gleiche Zeichenkette mit dem gleichen Trenner zerlegt. Das macht man nur *einmal* und merkt sich den immer gleichen Wert der dabei heraus kommt.
Das ``else: continue`` macht keinen Sinn. Das kann man ersatzlos streichen ohne das sich etwas am Programmablauf ändert. Mit ``continue`` sollte man sowieso sehr sparsam umgehen. Man kann Code eigentlich immer so strukturieren, dass man das nicht braucht.
Erst eine leere Liste anzulegen um gleich danach dann in zwei Anweisungen da per `append()` Elemente anzuhängen ist unnötig umständlich. Python hat Syntax um literale Listen mit Elementen zu schreiben, und dann kann man sich den Namen für das Zwischenergebnis auch gleich sparen:
Code: Alles auswählen
group_student_data = list()
group_student_data.append(parts[0])
group_student_data.append(parts[-1][:-1])
names_and_grades.append(group_student_data)
# =>
group_student_data = [parts[0], parts[-1][:-1]]
names_and_grades.append(group_student_data)
# =>
names_and_grades.append([parts[0], parts[-1][:-1]])
Das entfernen des Zeilenendezeichens über ``[:-1]`` ist gefährlich weil die letzte Zeile einer Textdatei das gar nicht zwingend haben muss. Es ist auch gar nicht nötig weil `float()` problemlos mit „whitespace“ vor und hinter der Zahldarstellung klar kommt:
Für das Zusammenfassen von Daten mit unterschiedlichen Bedeutungen je Index nimmt man Tupel und keine Listen.
Das beim schreiben der gleiche Dateiname verwendet wird ist blöd für's testen, weil man sich damit ja immer die Eingabedaten für den nächsten Testlauf überschreibt und deshalb vor jedem Testlauf die Eingabedaten erst wieder herstellen muss.
Auch beim Schreiben gilt wieder: ``with`` verwenden.
`f` ist kein guter Name. `file` würde viel besser aussagen was das ist.
Der Name `format_data()` triffts auch nicht wirklich gut, denn da werden die Daten nicht nur formatiert sondern überhaupt erst mal verarbeitet und dann auch noch in eine Datei geschrieben.
Statt jedes Element an den einen Namen `student` zu binden und dann im Schleifenkörper über ”magische” Indexwerte auf die beiden Bestandteile zuzugreifen, würde man das besser gleich beim ``for`` an zwei aussagekräftige Namen binden.
Mindestens mal das Zwischenergebnis `new` braucht man nicht an einen Namen binden, das kann man auch gleich einsetzen. `new` ist ja auch supernichtssagend.
Den ``%``-Operator würde ich nicht mehr zur Zeichenkettenformatierung verwenden. Es gibt die `format()`-Methode und noch besser f-Zeichenkettenliterale.
`round()` verwendet man nicht um die Darstellung auf eine Anzahl von Nachkommastellen zu berechnen. Eventuell wärst Du auch ein bisschen überrascht *wie* `round()` arbeitet:
Code: Alles auswählen
In [372]: round(5.55, 1)
Out[372]: 5.5
In [373]: round(5.45, 1)
Out[373]: 5.5
In [374]: round(5.35, 1)
Out[374]: 5.3
In [375]: round(5.25, 1)
Out[375]: 5.2
In [376]: round(5.15, 1)
Out[376]: 5.2
Sind das die von Dir erwarteten Ergebnisse?
Du willst statt `round()` die Zeichenkettenformatierung dafür verwenden.
In `calculate_average()` muss man `average` nicht haben. Ein Ergebnis das gleich in der nächsten Zeile mit ``return`` zurückgegeben wird, braucht man nicht an einen Namen binden.
`results` würde besser `total` heissen. Auf jeden Fall nicht Mehrzahl, denn das ist ja nur ein skalarer Wert. Und `result` stimmt nicht, weil es nicht das Ergebnis ist.
`TESTS` ist auch ein etwas irreführender Name, denn dahinter verbirgt sich kein Container-Objekt das Tests beinhaltet sondern die Anzahl der Tests. Aber ich glaube das hatte ich auch schon geschrieben: das sollte man nicht einfach so vorgeben und annehmen, weil sonst bei falschen Eingabedaten mit mehr oder weniger als drei Noten einfach ein falsches Ergebnis berechnet wird, ohne dass das jemandem auffallen muss. Diese Zahl sollte der jeweiligen *tatsächlichen* Anzahl der Noten entsprechen.
Statt an " " zu teilen würde man hier das Argument von `split()` einfach weg lassen. Das ist robuster.
Kleiner Zwischenstand:
Code: Alles auswählen
#!/usr/bin/env python3
def calculate_average(grades):
grades = grades.split()
total = 0
for grade in grades:
total += float(grade)
return total / len(grades)
def load_names_and_grades():
with open("grades.txt", encoding="ascii") as lines:
names_and_grades = []
for line in lines:
parts = line.split("_")
if parts:
names_and_grades.append([parts[0], parts[-1]])
return names_and_grades
def process_and_save_names_and_grades(names_and_grades):
with open("grades_2.txt", "w", encoding="ascii") as file:
file.write("Report.\n")
for name, grades in names_and_grades:
average_grade = calculate_average(grades)
file.write(
f"{name} has an average grade of {average_grade:.1f}.\n"
)
file.write("End of report.\n")
def main():
process_and_save_names_and_grades(load_names_and_grades())
if __name__ == "__main__":
main()
Die Aufteilung der Aufgaben auf die Funktionen ist aber eher schlecht. Daten sollten an der Stelle wo sie das Programm betreten in eine Form gebacht werden in der sie weiterverbeitet werden können. Aus einer Zeile sollte ein Paar aus Name und Noten werden, wobei die Noten eine Liste aus Zahlen sein sollten und keine Zeichenkette die erst weiter hinten in der Verarbeitung in Zahlen umgewandelt werden. Von einer Funktion die einen Durchschnitt ausrechnet erwartet jeder das man da die Zahlen übergibt, und keine Zeichenkette wo die Zahlen erst noch heraus gelesen werden müssen.
Die `calculate_average()`-Funktion wird auch supersimpel wenn man die Umwandlung von einer Zeichenkette in Zahlen da raus lässt, denn dann braucht man keine Schleife mehr, sondern kann einfach die `sum()`-Funktion verwenden.
Und am Namen `process_and_save_names_and_grades()` sieht man, dass die Funktion vielleicht zu viel macht. Übliche ”Trennlinien” sind die Eingabe der Daten in das Programm, die Verarbeitung, und die Ausgabe. Das kann man dann alles drei unabhängig voneinander Testen. Man kann Testdaten laden ohne das da was verarbeitet oder gespeichert werden muss. Man kann die Verarbeitung testen ohne Daten aus Dateien zu laden oder in Dateien zu speichern. Und man kann das schreiben von Daten in Dateien testen, ohne vorher Daten aus einer Datei geladen und verarbeitet zu haben. Das hat dann auch zur Folge, dass man Ein- und Ausgabe leicht tauschen oder wählbar machen kann. Denn der Verarbeitung ist es ja egal ob die Daten aus einer Datei kommen und welches Format die hat, oder aus einer Datenbank, oder einem Webdienst, und so weiter. Umgekehrt ist es für Eingabe und Verarbeitung auch egal wo die Daten am Ende hingeschrieben werden. Ob auf dem Bildschirm ausgegeben, in einer Datei (Text, PDF, als Diagramm in einem Bild, …), in einer Datenbank gespeichert, und so weiter.
Um die Funktionen flexibler zu machen sollten die Dateinamen Argumente ein. Dann kann man aus verschiedenen Dateien laden und in verschiedene Dateien speichern. Tests mit Testdaten werden so einfacher.
Nächster Zwischenstand mit einer sinnvolleren Aufteilung des Codes auf verschiedene Funktionen:
Code: Alles auswählen
#!/usr/bin/env python3
def calculate_average(grades):
return sum(grades) / len(grades)
def load_names_and_grades(filename):
with open(filename, encoding="ascii") as lines:
names_and_grades = []
for line in lines:
parts = line.split("_")
if parts:
name = parts[0]
grades = list()
for grade in parts[-1].split():
grades.append(float(grade))
names_and_grades.append((name, grades))
return names_and_grades
def calculate_averages(names_and_grades):
names_and_averages = list()
for name, grades in names_and_grades:
names_and_averages.append((name, calculate_average(grades)))
return names_and_averages
def save_report(filename, names_and_averages):
with open(filename, "w", encoding="ascii") as file:
file.write("Report.\n")
for name, average_grade in names_and_averages:
file.write(
f"{name} has an average grade of {average_grade:.1f}.\n"
)
file.write("End of report.\n")
def main():
save_report(
"grades_2.txt", calculate_averages(load_names_and_grades("grades.txt"))
)
if __name__ == "__main__":
main()
Jetzt kann man mal die Schleifen angehen und schauen aus welchen man einfach und sinnvoll „list comprehensions“ oder Generatorausdrücke machen kann:
Code: Alles auswählen
#!/usr/bin/env python3
def calculate_average(grades):
return sum(grades) / len(grades)
def load_names_and_grades(filename):
with open(filename, encoding="ascii") as lines:
names_and_grades = []
for line in lines:
parts = line.split("_")
if parts:
names_and_grades.append(
(parts[0], [float(grade) for grade in parts[-1].split()])
)
return names_and_grades
def calculate_averages(names_and_grades):
return [
(name, calculate_average(grades)) for name, grades in names_and_grades
]
def save_report(filename, names_and_averages):
with open(filename, "w", encoding="ascii") as file:
file.write("Report.\n")
file.writelines(
f"{name} has an average grade of {average_grade:.1f}.\n"
for name, average_grade in names_and_averages
)
file.write("End of report.\n")
def main():
save_report(
"grades_2.txt", calculate_averages(load_names_and_grades("grades.txt"))
)
if __name__ == "__main__":
main()
