Easy Learn - Vokabeltrainer

[Scriptinggamer]

Hallo,
Ich habe jetzt erstmal das Verwaltungsfenster eingebaut, mit einer Funktion zum Listen Löschen (sonst nichts).
Neustes Commit hier inwieweit ist das Ok?
Gruß

[Hyperion]

Deine Fenster habe ich mich jetzt nicht angeguckt, aber das API von Deiner Vokabelklasse ist ja (immer noch) grauenhaft:

Code: Alles auswählen

    def add_vocable(self):
        self.vocables.append({"vocable1" : "",
                              "vocable2" : "",
                              "crib" : ""})

    def change_vocable(self, index, vocable1, vocable2, crib):
        self.vocables[index] = {"vocable1" : vocable1,
                                "vocable2" : vocable2,
                                "crib" : crib}

# und dann zum Benutzen:
    vocabulary.add_vocable()
    vocabulary.change_vocable(0, "Hund", "dog", "Dogge")
    vocabulary.add_vocable()
    vocabulary.change_vocable(1, "Katze", "cat", "Kitekat")
    vocabulary.add_vocable()
    vocabulary.change_vocable(2, "Ratte", "rat", "ohne te(e)")

Das musst Du doch selber sehen, wie umständlich Du das gelöst hast!
Wieso kann `add_vocable` nicht die Argumente direkt aufnehmen? Und Dein `change_vocable` könnte doch dann einfach `delete_vocable` und `add_vocable` nutzen!

Zudem verstehe ich immer noch nicht, wieso Du das als Liste von Dictionaries ablegst. Wesentlich einfacher wäre es imho immer noch, das ganze als Dictionary anzulegen. Wenn Du darin Übersetzung(en) und Hinweiswort flexibel organisieren willst, dann nutze eben als Wert ein weiteres Dictionaries mit fixen Keys:

Code: Alles auswählen

vocabularies = {
    "eat" : {
        "translations": ["essen", "fressen"],
        "mnemonic": "Nicht trinken, sondern..."
    },
    "walk": { ... },
    ...
}

Wenn man wirklich mit solchen Zusatzinformationen arbeiten will, dann würde ich mir fast überlegen, eine einzelne Vokabel noch in einer Klasse zu modellieren.

Auf jeden Fall ist deine jetzige Form ziemlich umständlich.

Ein Index als Zugriff auf eine Vokabel ist an sich auch schon keine gute Idee, da sich bei Änderungen der Reihenfolge ggf. Abhängigkeiten verschieben! Bedenke doch mal, Du willst später mal "Lektionen" einführen, also Sammlungen von Vokabeln. Bei Deinem Ansatz wäre das so etwas wie eine Liste von Indizes:

Code: Alles auswählen

lesson = [0, 1, 2, 6, 10, 22, 34]

Nun musst Du beim Löschen einer Vokabel aufpassen, dass Du die Referenz auch aus der Lektion heraus nimmst, da sich ansonsten alle anderen Referenzen ja verschöben. Nehmen wir an, dass die Vokabel mit dem Index 6 gelöscht würde, dann würden nun die restlichen Vokabeln 10, 22 und 34 auf eine andere Vokabl verweisen, nämlich die bis dato jeweils danach kommende.

Würdest Du das Wort als Schlüssel nutzen, dann könntest Du bei der Lektion diesen einfach nutzen. Damit musst Du zwar aufpassen, ob das Wort noch existiert (`KeyError` kann man leicht abfangen und dazu nutzen!), aber alle *anderen* Wörter bleiben Dir genau so erhalten.

Weiterhin fiel mir noch dieses Konstrukt auf:

Code: Alles auswählen

            deleted = True
            for file_index in selection:
                try:
                    os.remove(os.path.join("vocabulary",
                                           mainwindow.files[file_index]))
                except WindowsError:
                    deleted = False
            if deleted:
                msg.showinfo(title = "Erfolgreich gelöscht",
                message = "Die ausgewählten Listen wurden erfolgreich gelöscht")
            else:
                msg.showinfo(title = "Fehler",
                         message = "Beim Löschen ist ein Fehler aufgetreten")

Das ist doch auch wieder so eine unnötige Sache! Wozu brauchst Du `deleted`? Doch nur, um *außerhalb* des `try...except`-Konstruktes nachträglich zu gucken, ob eine Ausnahme passiert ist oder nicht. Stattdessen kannst Du Dir das doch komplett sparen und die Aktionen eben *in* den Block reinziehen:

Code: Alles auswählen

            for file_index in selection:
                try:
                    os.remove(os.path.join("vocabulary",
                                           mainwindow.files[file_index]))
                    msg.showinfo(title = "Erfolgreich gelöscht",
                    message = "Die ausgewählten Listen wurden erfolgreich gelöscht")
                except WindowsError:
                    msg.showinfo(title = "Fehler",
                         message = "Beim Löschen ist ein Fehler aufgetreten")

Generell würde ich mir noch mal mehr Gedanken zu meiner Datenstruktur machen, bevor ich an der GUI weiterarbeiten würde. Und solltest Du nun noch daran etwas ändern, hast Du bestimmt einige Mühe beim Refactorn Deines GUI-Codes... und genau deswegen lohnt es sich, das im Vorfeld möglichst gut zu durchdenken

[deets]

Das ist alles immer noch murks. Hyperion hat ja schon eine Menge richtiges zu deinem immer noch falschen Ansatz ueber eine Liste gesagt.

Und ich habe dir vorgeschlagen, die *Sprache* als Schluessel zu benutzen. Also zb

Code: Alles auswählen

{ "deutsch" : "pferd", "englisch" : "horse" }

Dann brauchst du nicht deine Daten umzustrukturieren, nur weil du deutsch->englisch oder englisch->deutsch "umschaltest".

Aber dein "language1" "language2" -Kram ist einfach nur ein dictionary mit durchnummerierten Schluesseln - da kannst du auch bei deiner Liste bleiben, das aendert ja nix.

check_answer gibt "false" zurueck - das ist natuerlich auch nicht richtig, sondern muesste False (ohne Anfuehrungszeichen) sein. Und "upperLower" solltest du dir sparen, sondern gleich True zureckgeben, weil du es immer normalisiert vergleichen solltest.

Ausserdem sollte das IMHO eine Methode von Vocabulary sein, (welches immer noch keine new-style Klasse ist, das hatte BlackJack schon vor diversen Posts erwaehnt). Denn nur so kannst du dann zB die entsprechende Vokabel spaeter mal markieren als "richtig beantwortet", und die Wahrscheinlichkeit dafuer, dass sie beantwortet werden muss, reduzieren. So, wie gute Vokabeltraiener das machen.

Auch die mehrfachen shuffles/asks/replaces und so sind alle unnoetig. Einmal reicht.

Und das ewige rumgefuhrwerke mit "vocabulary" als Pfad ist auch unnoetig. Ein Vocabulary-Objekt sollte seinen vollen Pfad kennen, um sich selbst laden, speichern und loeschen zu koennen.

Last but not least solltest du die GUI in ein *extra* Modul packen, damit du dann getrennt testen und GUI starten kannst.

[deets]

Und noch ein kleiner Nachtrag:

Code: Alles auswählen

  if vocables == None:
            self.vocables = []
        else:
            self.vocables = vocables

Das ist ebenfalls nicht ideal. Zum einen sollten Vergleiche mit None mit "is" stattfinden, da None ein Singleton ist, und auf Objektidentitaet geprueft werden sollte. Das macht halt der is-Operator.

Zum anderen wuerde ich das kompakter schreiben:

Code: Alles auswählen

if vocables is None:
   vocables = []
self.vocables = vocables

Damit hast du den kompletten else-Zweig gespart, was von Vorteil ist da nun ein Betrachter (im Zweifel du selbst in 3 Wochen) weniger verstehen muss, was da vor sich geht.

[Scriptinggamer]

Hi,
-Ich könnte es folgender maßen machen:

Code: Alles auswählen

{"vocables" : [{"Deutsch" : ["hallo", "guten Tag", "hi"], "Englisch" : ["hello", "hi"]},
                   {"Deutsch" : ["Hund"], "Englisch" : ["dog"]}],
"languages" : ["Deutsch", "Englisch"]}

Das wäre zwar wieder in einer liste, aber es würde mehrere übersetzungen erlauben, in beiden Richtungen. Ich könnte ja ein Dictionary weder geordnet, noch geshuffelt benutzen, oder geht das irgendwie? ich könnte höchstens noch eine liste mit den key's erstellen, über die dann das dictionary angesprochen wird, aber das ist ja doof.
-Und ich könnte dann eben einfach die Sprachen umdrehen und damit die Abfragerichtung, nach deets Idee.
-Aber ich sollte doch nicht nur Richtig und Falsch benutzen, jetzt auf einmal doch? Soll ich dann erst für den Richtig-/Falsch-Dialog überprüfen was genau der Fehler ist?
-Was ist eine "new-style Klasse"?
-Naja das get_ask_vocables gibt erstmal eine liste zum Abfragen zurück, und das replace_ask_vocables shuffelt sie und sortiert aus, wie sollte das eine Funktion machen?
-das mit dem Pfad klingt gut
-Aber wenn ich es später cz_freezen will, muss dann nicht alles in einem script sein?
-Beim Löschen will ich ja nicht nach jeder Datei eine Meldung sondern erst wenn alle durch sind.
-Da ich immer direkt aus der Liste lösche, und dann die Listbox update, ist das mit den Indizes auch kein problem

Gruß

[Scriptinggamer]

Achja, eine Frage noch, wie soll ich den Pfad in der Datei speichern? Würde ein Aple oder Linux "vocaburary\\name.el" verstehen? Werden Ordner/Dateien da nicht anders getrennt?
Gruß

[CasualCoding.org]

Achja, eine Frage noch, wie soll ich den Pfad in der Datei speichern? Würde ein Aple oder Linux "vocaburary\\name.el" verstehen? Werden Ordner/Dateien da nicht anders getrennt?

Ja, mit einem slash: "vocabulary/name.el" - was praktischerweise auch unter Windows funktioniert.

[BlackJack]

@Scriptinggamer: Den Pfad solltest Du gar nicht in der Datei speichern. Warum sollte der dort gespeichert werden?

[Scriptinggamer]

alles klar, Denkfehler