Vokabeltrainer - eure Meinung !

Code-Stücke können hier veröffentlicht werden.
Antworten
Benutzeravatar
Manchotix
User
Beiträge: 54
Registriert: Samstag 14. Januar 2012, 19:54

Hallo Zusammen,

ich habe vor 2 tagen mal einen Vokabeltrainer geschrieben(noch nicht ganz fertig) und wollte euch denn Code mal vorstellen, damit ihr mir sagen könnt was ich so gut gemacht habe und vorallem was ich falsch gemacht habe.
Mir stellt sich auch die Frage ob der Code so richtig ist nach PEP 8 ? und wo noch verbesserungen gemacht werden können.

Bei der Kritik bitte nicht zu sanft sein (besonders wenn das ganze einfach nur Schlecht ist !)

Code: Alles auswählen

#!/usr/bin/python
# -*- coding: UTF-8 -*-
"""
autor = Manchotix
email = manchotix[at]googlemail.com
Homepage = bitbucket.org/Manchotix
TODO:
    Ordner abfrage - erstellung
    about erstellen
    hilfe erstellen
"""


import sys
import os
import pickle as p


_PATH = os.path.join(os.path.expanduser("~/" + ".vokapy/"))
_FILES = os.listdir(_PATH)


def train_session():
    """ Abfragen der Datein mit dem Listen object """

    for sessions in enumerate(_FILES):
        print("Sitzung {0} Thema: {1}".format(sessions[0], sessions[1]))

    number = int(input("Bitte, wählen sie einen Sitzung aus.\n>>>>>>> "))

    with open(_PATH + _FILES[number], "rb") as vok:
        vok_datei = p.load(vok)
        vok.close()

    for i in vok_datei:
        transl = str(input("{0} >>>>>>  ".format(i[0])))
        if transl in i:
            print("Richtig")
        else:
            print("Falsch, die Antwort ist: {0}".format(i[1]))


def creat_session():
    """Erstellen einer Datei die eine Listen Object enthält"""

    i = 0
    word_liste = []
    word_tup = ()

    
    name = str(input("Bitte, gebe einen Namen für deine Sitzung z.b. Farben, Essen ... \n >>>>>>>> "))
    while True:
        word_en = str(input("EN >>>>>>> "))
        word_de = str(input("DE >>>>>>> "))
        word_tup = (word1, word2)
        word_liste.insert(i, word_tup)
        i = i + 1
        if i == 10:
            break

    with open(_PATH + name + ".dat", "wb") as f:
        f.write(p.dumps(word_liste))
        f.close()


def main():
    """docstring"""

    while True:
        print("""

            Willkommen bei Vokapy,

            -------------- Menü -----------
            1: Starten einer Übung
            2: Erstellen einer Übung
            3: Hilfe
            4: Über denn Autor
            5: Verlassen / Exit

            """)

        menu_input = int(input(">>>>>  "))
        if menu_input == 1:
            train_session()
        elif menu_input == 2:
            creat_session()
        elif menu_input == 3:
            print("foobar3")
        elif menu_input == 4:
            print("foobar4")
        else:
            sys.exit("Auf wieder Sehen !")


if __name__ == '__main__':
    main()
- Über Fehler sollte man sich freuen als über das richtige Ergebnis denn wir Menschen können nur aus den Fehlern lernen-
mutetella
User
Beiträge: 1695
Registriert: Donnerstag 5. März 2009, 17:10
Kontaktdaten:

Hallo,

was mir jetzt mal bei Deiner `creat_session()` aufgefallen ist:

Der Rückgabewert von `input()` ist immer `str`, somit ist eine Umwandlung sinnfrei.
Die Namen `word_liste` und `word_tup` finde ich nicht gut.
Warum einen Zähler `i` in Verbindung mit `insert()`?
Was, wenn ich mehr oder weniger als 10 Eingaben machen möchte?

So in etwa fände ich es einfacher. Und statt `pickle` würde ich mir mal `json` ansehen. Damit bist Du letzten Endes flexibler.

Code: Alles auswählen

def create_session():
    words = []
    name = input('Name der Wortfamilie: ')
    print('Zum Beenden Felder leer lassen!\n')
    while True:
        en = input('EN > ')
        de = input('DE > ')
        if en and de:
            words.append((en, de))
        else:
            break
Und in etwa so, wie Du das Auswahlmenü in `train_session()` erstellst, würde ich es auch in der `main()` machen, mit einem dict....

mutetella
Entspanne dich und wisse, dass es Zeit für alles gibt. (YogiTea Teebeutel Weisheit ;-) )
Sirius3
User
Beiträge: 17737
Registriert: Sonntag 21. Oktober 2012, 17:20

Hallo Manchotix,

1. p ist kein schöner Name für pickle, weil er nichts sagt.
2. os.path.join ist hier überflüssig, genauso das zusammensetzen des Strings.
3. für _PATH und _FILES fallen Dir sicher noch bessere Namen ein
4. statt »sessions« solltest Du gleich zwei Variablen schreiben (session_num, session_name).
5. beim Zusammensetzen der Session-Datei benutzt Du plötzlich kein »os.path.join« mehr, obwohl es hier nötig wäre.
6. bei »with« ist »close« überflüssig
7. bei »i« erwartet der Leser eine einfache Zahl. Tupleunpacking wäre hier angebracht: (word_de, word_en)
8. »str« bei »input« ist unnötig, da »input« schon einen String liefert
9. »transl in i« ist wahrscheinlich nicht das was Du willst.
10. »creat_session« fehlt ein "e"
11. Die Funktion sofort umschreiben, so dass sie kein »i«, »word_tup« und »insert« mehr braucht.
12. siehe 5., 6. und 8.
13. es gibt »pickle.dump«

das reicht für's erste.
Benutzeravatar
Manchotix
User
Beiträge: 54
Registriert: Samstag 14. Januar 2012, 19:54

Ich danke euch schon mal,

werde denn Code mit euren Tipps nochmal überarbeiten und werde auf die Tipps auch in Zukunft achten.
mutetella hat geschrieben: Und statt `pickle` würde ich mir mal `json` ansehen. Damit bist Du letzten Endes flexibler.
Ok werde ich mal schauen, aber ist das an der stelle ein absoluter 'Fail' einsatz von Pickle und wenn ja wann sollte man denn Pickle verwenden ?
Habe es ja genommen damit ich einfach ein Liste in eine Datei speichern kann, weil mir das als eine recht gute Lösung vorgekommen ist.
- Über Fehler sollte man sich freuen als über das richtige Ergebnis denn wir Menschen können nur aus den Fehlern lernen-
BlackJack

@Manchotix: `pickle` beschränkt halt auf Python während es für JSON Bibliotheken für sehr viele verschiedene Programmiersprachen gibt. Und eine Liste, beziehungsweise die meisten Grunddatentypen von Python, lassen sich genau so einfach in Dateien speichern.
Benutzeravatar
Hyperion
Moderator
Beiträge: 7478
Registriert: Freitag 4. August 2006, 14:56
Wohnort: Hamburg
Kontaktdaten:

Und für 's Menü: Link ;-)
encoding_kapiert = all(verstehen(lesen(info)) for info in (Leonidas Folien, Blog, Folien & Text inkl. Python3, utf-8 everywhere))
assert encoding_kapiert
Benutzeravatar
Manchotix
User
Beiträge: 54
Registriert: Samstag 14. Januar 2012, 19:54

Das Menü in main habe ich schon umgeschrieben..

Code: Alles auswählen

menu_dict = {1: "train_session()", 
                 2: "creat_session()",
                 3: "foobar3",
                 4: "foobar4",
                 5: "sys.exit(0)"}
    
    while True:
        print("""
            Willkommen bei Vokapy,

            -------------- Menü -----------
            1: Starten einer Übung
            2: Erstellen einer Übung
            3: Hilfe
            4: Über denn Autor
            5: Verlassen / Exit
            """)

        menu_input = int(input(">>> "))
        if menu_input in menu_dict.keys():
            exec(menu_dict[menu_input])
        else:
            print("Nur Zahlen von 1 - 5 eingeben !")
Ich hoffe mal das ich das nicht falsch verstanden habe mit dem Dict oder ob das überhaupt eine gute idee ist wie es jetzt ist :/
mutetella hat geschrieben:Und in etwa so, wie Du das Auswahlmenü in `train_session()` erstellst, würde ich es auch in der `main()` machen, mit einem dict....
- Über Fehler sollte man sich freuen als über das richtige Ergebnis denn wir Menschen können nur aus den Fehlern lernen-
Sirius3
User
Beiträge: 17737
Registriert: Sonntag 21. Oktober 2012, 17:20

@Manchotix: ich weiß jetzt wirklich nicht, wie Du auf die Idee mit »exec« kommst. Hast Du den Link von Hyperion bis zum Ende gelesen?
Benutzeravatar
Manchotix
User
Beiträge: 54
Registriert: Samstag 14. Januar 2012, 19:54

@Sirius3
Noch nicht ganz wollte halt nur das aktuelle zeigen.
Habe dort exec verwendet weil ich dachte ich soll ein Dict verwenden und mir gerade keine andere lösung dazu eingefallen ist. :/
Aber ich denke mal exec und eval sollte man nicht unbedingt verwenden oder ? weil man ja auch sagt "eval ist evil !"
- Über Fehler sollte man sich freuen als über das richtige Ergebnis denn wir Menschen können nur aus den Fehlern lernen-
mutetella
User
Beiträge: 1695
Registriert: Donnerstag 5. März 2009, 17:10
Kontaktdaten:

Manchotix hat geschrieben:Habe dort exec verwendet weil ich dachte ich soll ein Dict verwenden ...
Ich persönlich verwende bei solchen Menüsachen gerne ein `dict`, in dem Tutorial von Hyperion findest Du eine Lösung über 'ne Liste. Letztlich auch ein wenig Geschmacksache, allerdings ist der Einsatz von `exec` an der Stelle völlig falsch!

Folgendes Beispiel bringt Dich sicherlich weiter:

Code: Alles auswählen

>>> def any_func():
...     return "Calling 'any_func'"
... 
>>> func = any_func
>>> print(func)
<function any_func at 0x7f6d6ae63270>
>>> print(func())
Calling 'any_func'
Damit lässt sich dann z. B. sowas machen:

Code: Alles auswählen

>>> menu = {1: any_func}
>>> print(menu[1])
<function any_func at 0x7f6d6ae63270>
>>> print(menu[1]())
Calling 'any_func'
mutetella
Entspanne dich und wisse, dass es Zeit für alles gibt. (YogiTea Teebeutel Weisheit ;-) )
Benutzeravatar
Hyperion
Moderator
Beiträge: 7478
Registriert: Freitag 4. August 2006, 14:56
Wohnort: Hamburg
Kontaktdaten:

mutetella hat geschrieben:Ich persönlich verwende bei solchen Menüsachen gerne ein `dict`, in dem Tutorial von Hyperion findest Du eine Lösung über 'ne Liste. Letztlich auch ein wenig Geschmacksache, ...
Stimmt im großen und ganzen. Wenn man einen numerischen, fortlaufenden Index hat, so braucht man einfach kein Dictionary. Letztlich gibt es dann afaik keinen Vorteil eines Dicts gegenüber einer Liste (in Python!).

In anderen Sprachen kann es sogar ein Nachteil sein, bei einem solchen Schlüssel einen Key-Value-Container zu verwenden. In C++ verwendet std::map iirc keine Hash-Funktion, sondern eine binäre Suche; ergo fällt der Zugriff auf ein Element schlechter aus, als bei einem Vector mit Indexzugriff.

@Manchotix: Deine Verbesserung ist ehrlich gesagt eher gering. Die Bezeichnungen der Menüeinträge ist immer noch semantisch entkoppelt von der Funktionalität eines Menüpunktes. Das würde ich ändern und Name + Funktion in *eine* Struktur packen.
encoding_kapiert = all(verstehen(lesen(info)) for info in (Leonidas Folien, Blog, Folien & Text inkl. Python3, utf-8 everywhere))
assert encoding_kapiert
Benutzeravatar
Manchotix
User
Beiträge: 54
Registriert: Samstag 14. Januar 2012, 19:54

So ich habe es jetzt mal überarbeitet ich hoffe das kein Punkt vergessen wurde der geändert werden sollte.

Code: Alles auswählen

#!/usr/bin/python
# -*- coding: UTF-8 -*-
"""  DocString """

__author__ = "Manchotix <manchotix@gmail.com> <bitbucket.org/Manchotix>"

import sys
import os
import json


user_dir = os.path.expanduser("~/.vokapy/")
items_of_dir = os.listdir(user_dir)


def train_session():
    for index, datei in enumerate(items_of_dir, 1):
        print("Sitzung {0} Thema: {1}".format(index, datei))
    choice = int(input("Wählen einen Sitzung\n>>> ")) -1
    with open(os.path.join(user_dir, items_of_dir[choice]), "r") as vok:
        vok_datei = json.load(vok)
    for vok_paar in vok_datei:
        transl = str(input("{0} >>>>>>  ".format(vok_paar[0])))
        if transl in vok_paar:
            print("Richtig")
        else:
            print("Falsch, die Antwort ist: {0}".format(vok_paar[1]))


def create_session():
    words = []
    name = input("Namen der Wortfamilie z.b. Farben, Essen ... \n>>> ")
    while True:
        en = input("EN >> ")
        de = input("DE >> ")
        if en and de:
            words.append((en, de))
        else:
            break
    with open(os.path.join(user_dir, "{}.json".format(name)), "w") as f:
        json_obj = json.dumps(words)
        f.write(json_obj)


def main():
    menu = [
        ["Starten einer Übung", train_session],
        ["Erstellen einer Übung", create_session],
        ["Verlassen/Exit", sys.exit]
    ]
    while True:
        for index, item in enumerate(menu, 1):
            print("{}: {}".format(index, item[0]))
        try:
            choice = int(input(">>> ")) - 1
            if 0 <= choice < len(menu):
                menu[choice][1]()
            else:
                print("\nNur Zahlen von 1-{}!\n".format(len(menu)))
        except ValueError:
            print("\nNur Zahlen eingeben !\n")


if __name__ == "__main__":
    main()
Das was ich jetzt noch dazu schreiben will ist das ich eine Check Funktion schreibe wo überprüft wird ob der Ordner vorhanden ist und wenn nicht soll er halt erstellt werden.
- Über Fehler sollte man sich freuen als über das richtige Ergebnis denn wir Menschen können nur aus den Fehlern lernen-
Benutzeravatar
/me
User
Beiträge: 3555
Registriert: Donnerstag 25. Juni 2009, 14:40
Wohnort: Bonn

Manchotix hat geschrieben:Das was ich jetzt noch dazu schreiben will ist das ich eine Check Funktion schreibe wo überprüft wird ob der Ordner vorhanden ist und wenn nicht soll er halt erstellt werden.
Ich halte es für pythonischer einfach die Erstellung durchzuführen und im Fehlerfall passend zu reagieren.
Antworten