Gebt mir Tipps

Wenn du dir nicht sicher bist, in welchem der anderen Foren du die Frage stellen sollst, dann bist du hier im Forum für allgemeine Fragen sicher richtig.
Mr.White
User
Beiträge: 46
Registriert: Samstag 7. März 2015, 20:03

Hallo,

Ich habe schon viel Kritik bekommen. Egal. Ich habe die letzten Stunden vor meinem Buch gehangen und ein kleines "BANK-PRG" programmiert.
Vielleicht könnt ihr mir sagen was ich verändern sollte. VORSICHT: Keine Kommentare benutzt.


Hier ist mein "Kleiner" Code. Ein Freund hat mir schon was vom Auslagern der Grafik oder so erzählt.Aber Davon hab ich noch kein Plan.
Bitte sehr:

Code: Alles auswählen


import time

class Konto:
    def __init__(self,kstand,kontonummer,pin):
        self.kontostand = kstand
        self.ID = kontonummer
        self.PW = pin
        
    def get_Kontostand(self):
        return self.kontostand

    def get_ID(self):
        return self.ID

    def get_PW(self):
        return self.PW


Konto1 = Konto(500, 356999, 8698)
Konto2 = Konto(700, 344586, 1134)


k_logged = 0

while k_logged == 0:
    print("Gebe deine ID ein:")
    k_id = int(input())
    if k_id == Konto1.get_ID() or k_id == Konto2.get_ID():
        print("PW eingeben:")
        k_pw = int(input())
        if k_pw == Konto1.get_PW() or k_pw == Konto2.get_PW():
            print("Du bist eingeloggt:")
            print()
            if k_id == Konto1.get_ID() and k_pw == Konto1.get_PW():
                print("Kontostand:" + str(Konto1.get_Kontostand()))
                print()
                print("Möchtest du eine Aktion ausführen ?:")
                a_dec = input()
                if a_dec == "Ja" or "yes":
                    print("")
                    print("Mögliche aktionen:")
                    print("- Überweisen")
                    a_dec = input()
                    if a_dec == "Überweisen":
                        print("Wie viel soll überwiesen werden ?:")
                        a_dec = float(input())
                        if a_dec >0:
                            print("An welche ID:")
                            print()
                            print("Mögliche ID's " + str(Konto2.get_ID()))
                            a_dec = int(input())
                            if a_dec == Konto2.get_ID():
                                print("Geld würde Überwiesen...")
                                ###Problem
                                #Wird noch nachgeholt :D
                                print("Neuer Kontostand: " + str(Konto1.get_Kontostand()))
                                k_logged = 1
                            elif a_dec != Konto2.get_ID():
                                print("Konto existiert nicht, oder du versucht dir selber was zu geben!")
                            else:
                                print("Error 101")
                        else:
                            print("Der betrag muss größer sein...")
                    elif a_dec != "Überweisen":
                        print("Falsche eingabe..")
                        print("Abbruch")
                    else:
                        print("ERROR 101..")
                elif a_dec == "Nein" or "no":
                    print("Dann nicht..")
            else:
                print("Erroroit")
            elif k_id == Konto2.get_ID() and k_pw == Konto2.get_PW():
                print("Kontostand:" + str(Konto2.get_Kontostand()))
                print()
                print("Möchtest du eine Aktion ausführen?:")
                a_dec = input()
                if a_dec == "Ja" or "yes":
                    print("")
                    print("Mögliche aktionen:")
                    print("- Überweisen")
                    a_dec = input()
                    if a_dec == "Überweisen":
                        print("Wie viel soll überwiesen werden ?")
                        a_dec = float(input())
                        if a_dec > 0:
                            print("An welche ID ?")
                            print("Mögliche ID's: " + str(Konto1.get_ID()))
                            a_dec = int(input())
                            if a_dec == Konto1.get_ID():
                                print("Das Geld wurde überwiesen...")
                                ##Hier auch :)
                                k_logged = 1
                            elif a_dec != Konto1.get_ID():
                                print("Diese ID exisitert nicht...")
                            else:
                                print("Du kannst dir nichts selber überweisen")
                        else:
                            print("Es muss eine Zahl sein die größer als 0 ist!")
                    elif a_dec != "Überweisen":
                        print("Falsche eingabe...")
                        print("Abbruch!")
                    else:
                        print("ERROR 101")
                elif a_dec == "Nein" or "no":
                    print("Dann nicht. Bis dann :)")
                else:
                    print("Mögliche eingaben - Ja und Nein.")
                    print("Programm wird hier abgebrochen")
                k_logged = 1
            else:
                print("Error")
        elif k_pw != Konto1.get_PW() or k_pw != Konto2.get_PW():
            print("Falsche eingaben.. Neuer versuch:")
            k_logged = 0
    elif k_id != Konto1.get_ID() or k_id != Konto2.get_ID():
        print("Falsche eingaben.. erneuter versuch:")
        k_logged = 0
    else:
        print("Falsche eingaben...")
        k_logged = 0



Ich weiß einer sagt: Das kann man sehr viel Kürzer gestalten. Verratet mir bitte wie.
Ist jetzt mein erstes richtiges Prog. Danke :D
Zuletzt geändert von Mr.White am Samstag 4. April 2015, 21:40, insgesamt 1-mal geändert.
Sirius3
User
Beiträge: 18255
Registriert: Sonntag 21. Oktober 2012, 17:20

@Mr.White: der erste Schritt, um das Programm kürzer zu machen ist, die ganzen get_xxx-Methoden zu löschen. Die braucht niemand weil man viel einfacher auf die Attribute direkt zugreifen kann.

Als nächstes solltest Du etwas über Listen lernen, steht bestimmt in Deinem Buch. Wie glaubst Du sehen Programme von richtigen Banken mit 1000000 Konten aus? Immer wenn Du anfängst, Variablennamen durchzunummerieren, nimm eine Liste. Immer wenn Du anfängst, Code zu kopieren, mach eine Funktion.
Mr.White
User
Beiträge: 46
Registriert: Samstag 7. März 2015, 20:03

Sirius3 hat geschrieben:@Mr.White: der erste Schritt, um das Programm kürzer zu machen ist, die ganzen get_xxx-Methoden zu löschen. Die braucht niemand weil man viel einfacher auf die Attribute direkt zugreifen kann.

Als nächstes solltest Du etwas über Listen lernen, steht bestimmt in Deinem Buch. Wie glaubst Du sehen Programme von richtigen Banken mit 1000000 Konten aus? Immer wenn Du anfängst, Variablennamen durchzunummerieren, nimm eine Liste. Immer wenn Du anfängst, Code zu kopieren, mach eine Funktion.

Hmm ja an listen dachte ich auch.. Muss ich mir mal anschauen.
Mr.White
User
Beiträge: 46
Registriert: Samstag 7. März 2015, 20:03

So, habe jetzt eine Liste die Stand, Pin und Kontonummer enthält.
Wie kann ich nun überprüfen ob man genau den selben Pin eingegeben hat ?


Damit habe ich grade probleme
BlackJack

@Mr.White: Es ging darum die durchnummerierten Namen loszuwerden, also *Konten* in eine Liste zu stecken. Nicht die Attribute eines Kontos in einer Liste zusammen zu werfen.
Mr.White
User
Beiträge: 46
Registriert: Samstag 7. März 2015, 20:03

Ich habe in der Init eine Liste welche die Daten des Kontos beinhaltet. Sprich ID, PW, Kontostand ?
ID und PW sind zufällig also kann ich auch mehrere Konten haben und ein Objekt erstellen welche diese
Eigenschaften aufweist und möglicherweise Speichert und aufruft. Das ist mein Ziel.
Ich weiß jetzt nicht was daran falsch ist ?
BlackJack

@Mr.White: Daran ist falsch das in dieser Liste völlig verschiedenartige Dinge stehen die jeweils anders behandelt werden während Liste zum speichern von gleichartigen Dingen mit denen man die gleichen Operationen durchführen möchte gedacht sind. Und das man jetzt auf die einzelnen Bestandteile eines Kontos statt über beschreibende Namen über nichtssagende ”magische” Indexzahlen zugreifen muss. Apropos beschreibende Namen: wenn ein Attribut eine Kontonummer darstellt und ein anderes eine Pin, dann sollten die auch so heissen und nicht `ID` und `PW`.
Mr.White
User
Beiträge: 46
Registriert: Samstag 7. März 2015, 20:03

Ok ich überleg mir was
BlackJack

@Mr.White: Der erste Verbesserungsvorschlag hätte wohl sein müssen den Syntaxfehler zu beheben der verhindert dass das überhaupt kompiliert. :-)
EyDu
User
Beiträge: 4881
Registriert: Donnerstag 20. Juli 2006, 23:06
Wohnort: Berlin

Und

Code: Alles auswählen

x == "Ja" or "yes"
macht sicher nicht das, was du erwartest. Gib doch einfach mal etwas anderes ein als "Ja" oder "yes". Vielleicht mal "No" oder gar nichts. Kleiner Tipp: Programme werde nicht in Umgangssprache geschrieben und Strings haben eine lower-Methode.
Das Leben ist wie ein Tennisball.
BlackJack

Das Hauptprogramm sollte in einer Funktion stecken, üblicherweise `main()` genannt, und durch folgendes Idiom aufgerufen werden:

Code: Alles auswählen

if __name__ == '__main__':
    main()
Dann kann man das Modul importieren ohne dass das Programm ausgeführt wird. Zum Beispiel um einzelne Funktionen oder Klassen interaktiv in einer Shell zu testen, automatisierte Tests auszuführen, und auch einige andere Werkzeuge erwarten das man Module ohne Nebeneffekte importieren kann.

Das Hauptprogramm ist viel zu lang und dadurch zu unübersichtlich. Das solltest Du sinnvoll auf Funktionen verteilen. In der Regel sind Funktionen und Methoden maximal 25 bis 50 Zeilen lang (ohne Docstring). Sonst wird es unübersichtlich.

`time` wird importiert aber nicht genutzt.
Benutzeravatar
Hyperion
Moderator
Beiträge: 7478
Registriert: Freitag 4. August 2006, 14:56
Wohnort: Hamburg
Kontaktdaten:

Ich verweise auch noch mal auf den Ur-Thread. Da sind Dir auch schon ettliche gute Tipps gegeben worden.
encoding_kapiert = all(verstehen(lesen(info)) for info in (Leonidas Folien, Blog, Folien & Text inkl. Python3, utf-8 everywhere))
assert encoding_kapiert
BlackJack

Anstelle der `get*`-Methoden kann man direkt auf die Attribute von `Konto`-Objekten zugreifen. Und die sollten die Namen haben die man beim Konto erwartet: `kontonummer` und `pin`.

Die Konto-Objekte sind statt in einer Liste wahrscheinlich besser in einem Wörterbuch (`dict`) aufgehoben. Du greifst über die PIN als erstes auf die Konten zu, aber es wäre sinnvoller das über die Kontonummer zu machen denn die ist eindeutig während verschiedene Benutzer ja durchaus die gleiche PIN haben können.
Benutzeravatar
bwbg
User
Beiträge: 407
Registriert: Mittwoch 23. Januar 2008, 13:35

Ich ginge ergänzend soweit, die Kontonummer nicht als Attribut des Kontoobjektes zu verwenden. Stattdessen würde die Kontonummer als Schlüssel des von BlackJack genannten dictionary verwendet.

Die Authentifizierung ließe sich wie folgt umsetzen:

Code: Alles auswählen

#!/usr/bin/env python3
# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
# accounting.py
# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -

class Account:
    def __init__(self, title, pin=None):
        self.title = title
        self.pin = pin
        
    def __repr__(self):
        return '<Account(title={}, pin={})>'.format(repr(self.title),
                                                    repr(self.pin))

        
def authenticate(accounts, account_key, pin):
    """Authenticates the `account_key`, `pin` pair against the `accounts`.
    Returns `account_key` if authorization was successfull otherwise `None`.
    """
    account = accounts.get(account_key, Account(''))
        # The default-value of dict.get returns a Dummy-account which will
        # always fail to authenticate (pin == None).
    return account_key if account.pin == pin and account.pin is not None \
                       else None

def main():
    """Program's entry-point."""
    accounts = {
        'A-1200': Account('Bank (Cash)'), # Internal bank-account, no login
        'C-1001': Account('Customer: John Doe', 'jd'),
        'C-1002': Account('Customer: Joanne Dowinski', 'jnd'),
    }
    
    assert authenticate(accounts, 'A-1200', 'wrong') == None
    assert authenticate(accounts, 'A-1200', None) == None
    assert authenticate(accounts, 'C-1002', 'jnd') == 'C-1002'
    assert authenticate(accounts, 'C-1001', 'wrong') == None

if __name__ == '__main__':
    main()
Den Kontostand kann man in der jetzigen Version durchaus noch im Kontoobjekt belassen. Sobald man aber auf die Idee kommt, Kontoauszüge bereitstellen zu wollen, sollte man den Kontostand als Summe über alle Transaktionen welche das Konto betreffen definieren.
"Du bist der Messias! Und ich muss es wissen, denn ich bin schon einigen gefolgt!"
BlackJack

@bwbg: Die Kontonummer ist doch aber eine Eigenschaft des Kontos. Das man die ausserhalb des Kontos noch irgendwo verwendet, ist doch kein Grund sie nicht auch im Konto-Objekt vorzuhalten wo sie hingehört.

Edit: Die `authenticate()`-API finde ich komisch: Warum gibt mir die Funktion etwas zurück das ich bereits als Argument übergeben habe? Ich würde da `True` oder `False` erwarten. Oder das passende `Account`-Objekt oder `None` beziehungsweise eine Ausnahme.
Benutzeravatar
bwbg
User
Beiträge: 407
Registriert: Mittwoch 23. Januar 2008, 13:35

1. Kontonummer-Attribut

Ich sehe da ja auch einen Zusammenhang, allerdings nur einen mittelbaren (durch die Zuordnung). Zudem möchte ich Redundanzen als Fehlerquellen vermeiden. Wenn die Kontonummer in das Kontoobjekt aufgenommen würde, dürfte sie nicht erneut (redundant) als Schlüssel des dictionaries verwendet werden. Die Frage ist doch, ob das Kontoobjekt die ihm Container übergeordnete Zuordnung (hier dict) kennen muss. Das sehe ich im Moment nicht.

Alternativ böte sich ein set an. Wobei man hier die Kontoklasse (nun mit Kontonummer) und entsprechende Funktionen set-tauglich gestalten müsste.

2. authenticate()-API

Der Gedanke war, dass die API durch ein BankOMat-Objekt, welches die Benutzerschnittstelle definiert, verwendet wird. Dieses Objekt verwendet die Kontonummer als inneren Zustand. Kontonummer vorhanden -> angemeldet, Kontonummer nicht vorhanden (=None) -> nicht angemeldet.

Zugegeben, so sollte man produktiv nicht mit Sicherheit umgehen ;)
"Du bist der Messias! Und ich muss es wissen, denn ich bin schon einigen gefolgt!"
BlackJack

@bwbg: Ad 1.) Eine Kontonummer gehört zu einem Konto-Objekt, das ist eine Eigenschaft des Kontos, dadurch kann man ein Konto innerhalb einer Bank von anderen Konten unterscheiden. Wenn ich ein Konto-Objekt habe und von dem nicht seine Kontonummer abfragen kann, dann fehlt da etwas. Das hat überhaupt nichts damit zu tun wie ein `Konten`- oder `Bank`-Objekt die `Konto`-Objekte am Ende verwaltet. Wie Du schon sagst, davon sollte ein Konto-Objekt nichts wissen müssen und man sollte nicht wegen Implementierungsdetails wichtige Attribute die zur Problemdomäne gehören einfach weglassen.

Ad 2.) Warum der unnötige Umweg? Warum nicht `Konto`-Objekt vorhanden = angemeldet, `None` = nicht angemeldet? Und selbst Dein Ansatz erklärt immer noch nich die API, denn das liesse sich auch mit einem Wahrheitswert als Rückgabwert der `authenticate()`-Funktion realisieren. Wenn da `True` zurück kommt setzt man die Kontonummer als inneren Zustand, falls `False` zurück kommt eben `None`. Die Kontonummer hat der Aufrufer ja, weshalb ich die API ja schräg finde das die Kontonummer die man sowieso schon hat, als Rückgabwert kommt oder halt `None` was letztlich auf zwei Mögliche Rückgabewert hinaus läuft, also eigentlich ein Wahrheitswert.
Benutzeravatar
bwbg
User
Beiträge: 407
Registriert: Mittwoch 23. Januar 2008, 13:35

@BlackJack:
Zu 1) gebe ich Dir recht. Wenn man intensiver darüber nachdenkt, ist es sinnfrei, die Kontonummer getrennt vom Kontoobject zu halten.

Zu 2) bin ich noch nicht ganz überzeugt. None eignet sich doch hervorrangend als (mögliches) Ergebnis für eine Operation, welche fehlschlagen kann. Zumal sich das Ergebnis (durch None) als Wahrheitswert verwenden ließe.

Code sagt mehr als tausend Worte: http://www.python-forum.de/pastebin.php?mode=view&s=429
Anmerkung: Account verfügt (noch) nicht über ein account_id-Attribut.

Hier verwendet die BankOMat.login das von mir vorgeschlagene API-Konstrukt. Die Rückgabe als Wahrheitswert oder gar eine Exception würde das ganze m. E. nach nicht übersichtlicher machen.
"Du bist der Messias! Und ich muss es wissen, denn ich bin schon einigen gefolgt!"
Sirius3
User
Beiträge: 18255
Registriert: Sonntag 21. Oktober 2012, 17:20

@bwbg: ein

Code: Alles auswählen

if self.bank.authenticate(account_id, password):
    self.account_id = account_id
würde den Code um einiges klarer machen. In Deiner Schreibweise würde sich jeder Leser wundern, wie das Argument account_id mit dem Rückgabewert zusammenhängt. Mit if ist klar, dass es keine zwei verschiedenen account_ids gibt.
BlackJack

@bwbg: Hm, der Entwurf wird IMHO nicht besser. Transaktionen gehören IMHO zum Beispiel zum Konto und nicht zur Bank. Dein Konto-Objekt enthält ja so gut wie gar nichts, dabei wäre das aus Sicht eines Kunden ein zentrales Objekt auf dem so gut wie alles relevante erreichbar ist. Der Titel ist eigentlich eine Verquickung von Kontotyp und Benutzer. Wenn man schon Benutzer und Pins hat, dann würde ich auch eher sagen das Name und Pin in einer `User`-Objekt gehören. Wenn man es einfach halten möchte dann wäre der Kontotyp eine beschreibende Zeichenkette, oder man würde das wenn man die Typen tatsächlich unterscheiden muss über verschiedene Klassen regeln oder das Strategie-Entwurfsmuster anwenden. Der regelmässige Abschluss zum Beispiel sieht ja bei Girokonten anders aus als bei Sparkonten oder internen Konten.
Antworten