Mugen hat geschrieben:
Ließe sich das denn noch anders lösen?
Ja klar, indem Du das Dictionary-Objekt einfach an die Funktionen übergibst
Nehmen wir das Beispiel ``save_new_number``. Die Signatur der Funktion sähe dann eben so aus:
Das kannst Du dann aus Deinem Menü mit dem Objekt als Parameter aufrufen.
Einige Anmerkungen zum Code:
- Es ist imho ungewöhnlich, dass eine ``main``-Funktion ganz oben in der datei steht. IdR. erwarte ich die unten direkt über dem ``if __name__``-Hook.
- Wieso merkst Du Dir die Menüeinträge nicht auch in Deinem Dispatcher-Dict? z.B. so:
Code: Alles auswählen
choices = {
'S': (search_number, (S)uche nach Telefonnummer),
'N': (save_new_number, "(N)eue Nummer eintragen"),
'A': (print_numbers, "(A)lle Nummern ausgeben"),
'E': (sys.exit, "(E)nde")
}
Damit hast Du alles in einer Datenstruktur und musst beim Hinzukommen / einer Änderung eines Menüpunktes nicht eine ``print``-Zeile hinzufügen
Wenn es Dir auf die Reihenfolge ankommt, dann kannst Du z.B. ein ``OrderedDict`` benutzen - denn bei einem normalen Dictionary werden die Menüpunkte natürlich nicht in der gegebenen Reoihenfolge ausgegeben. Alternativ verzichtest Du auf das Dictionary und nimmst generell ein Tupel, oder legst Dir ein Tupel der Keys für die Reihenfolge separat an.
- Beim Dispatching kannst Du Dir das Überschreiben von ``choice`` sparen:
Code: Alles auswählen
# Deine Lösung
try:
choice = choices[choice]
choice()
except KeyError:
print("Auswahl nicht erkannt")
print()
continue
# imho hübscher:
try:
choices[choice]()
except KeyError:
print("Auswahl nicht erkannt", end="\n\n")
- Generell mischst Du die Verwendung von ``print()`` für eine Leerzeile und das Anhängen eines "\n". Das würde ich vereinheitlichen.
- ``search_number`` kann so doch nicht klappen.
Mir diesem Code bekommst Du niemals die Nummer der gesuchten Person
Genau an dieser Stelle kann man sich auch streiten, ob man mit einem ``if something in dict``-Konstrukt sicher stellt, dass ein Name im Telefonbuch enthalten ist, oder es mittels ``try... except KeyError`` löst, wie Du das beim Dispatchen oben tust.
- Zeile 54 kann man kürzen:
Code: Alles auswählen
# bei dir
for i in tel.keys():
# kompakter
for name in tel:
- Namen wie ``i`` sind auch nicht besonders toll. Wieso nimmst Du an der Stelle nicht ``name`` als Namen? Genau das repräsentiert ja der Schlüssel in Deinem Telefonbuch!
- Wieso schreibst Du kein leeres Dict in die Datei bei der Fehlerbehandlung in ``print_numbers``? Taucht denn da gar kein Fehler beim Laden auf in Zeile 51:
Code: Alles auswählen
with open("tel.txt", "rb") as tel:
tel = pickle.load(tel)
Aus einer leeren Datei kann ``pickle`` ja eigentlich nur ein ``None``-Object generieren - das wäre für den weiteren ABlauf aber fatal.
Generell sieht das alles gar nicht so verkehrt aus. Aber: Bist Du Dir darüber im klaren, dass Du immer auf Dateiebene arbeitest? Bei jedem Eintrag in Dein Telefonbuch speicherst Du das Telefonbuch-Dict und bei jeder Anzeige lädst Du das Objekt wieder in den Speicher. Ist das sinnvoll? Beim Neuanlegen ist das vielleicht sogar eine gute Idee, damit man Änderungen nicht immer separat speichern muss (per zudätzlichem Menüpunkt). Aber beim Anzeigen der Datensätze, musst Du das doch nicht neu laden?! Du hast die Daten ja im Speicher
Zumal Du beim Suchen da "inkonsequent" bist und tatsächlich in Deinem Objekt suchst.
Zudem hast Du den Dateinamen ``tel.txt`` somit als "magic number" fest in Deinem Code in jeder Funktion. Imho ist es da sinnvoller, die Dateioperationen von den eigentlichen Datenoperationen zu separieren und zudem den Dateinamen als Parameter an diese IO-Funktionen zu übergeben. Wenn Du ihn nicht per Kommandozeilenparameter übergeben magst, kannst Du den tatsächlich auf Modulebene festlegen:
(Wobei ".txt" nicht so glücklich ist bei einer ``pickle``-Datei - die ist ja binär!)
