Seite 1 von 2

Gebt mir Tipps

Verfasst: Samstag 4. April 2015, 21:34
von Mr.White
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

Re: Gebt mir Tipps

Verfasst: Samstag 4. April 2015, 21:39
von Sirius3
@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.

Re: Gebt mir Tipps

Verfasst: Samstag 4. April 2015, 21:41
von Mr.White
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.

Re: Gebt mir Tipps

Verfasst: Samstag 4. April 2015, 22:21
von Mr.White
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

Re: Gebt mir Tipps

Verfasst: Samstag 4. April 2015, 22:42
von 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.

Re: Gebt mir Tipps

Verfasst: Samstag 4. April 2015, 22:48
von Mr.White
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 ?

Re: Gebt mir Tipps

Verfasst: Samstag 4. April 2015, 23:05
von 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`.

Re: Gebt mir Tipps

Verfasst: Samstag 4. April 2015, 23:06
von Mr.White
Ok ich überleg mir was

Re: Gebt mir Tipps

Verfasst: Montag 6. April 2015, 00:37
von BlackJack
@Mr.White: Der erste Verbesserungsvorschlag hätte wohl sein müssen den Syntaxfehler zu beheben der verhindert dass das überhaupt kompiliert. :-)

Re: Gebt mir Tipps

Verfasst: Montag 6. April 2015, 14:52
von EyDu
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.

Re: Gebt mir Tipps

Verfasst: Dienstag 7. April 2015, 09:37
von 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.

Re: Gebt mir Tipps

Verfasst: Dienstag 7. April 2015, 14:41
von Hyperion
Ich verweise auch noch mal auf den Ur-Thread. Da sind Dir auch schon ettliche gute Tipps gegeben worden.

Re: Gebt mir Tipps

Verfasst: Mittwoch 8. April 2015, 00:14
von 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.

Re: Gebt mir Tipps

Verfasst: Mittwoch 8. April 2015, 09:39
von bwbg
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.

Re: Gebt mir Tipps

Verfasst: Mittwoch 8. April 2015, 10:11
von 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.

Re: Gebt mir Tipps

Verfasst: Mittwoch 8. April 2015, 10:49
von bwbg
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 ;)

Re: Gebt mir Tipps

Verfasst: Mittwoch 8. April 2015, 11:01
von 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.

Re: Gebt mir Tipps

Verfasst: Mittwoch 8. April 2015, 13:40
von bwbg
@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.

Re: Gebt mir Tipps

Verfasst: Mittwoch 8. April 2015, 14:02
von Sirius3
@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.

Re: Gebt mir Tipps

Verfasst: Mittwoch 8. April 2015, 14:10
von 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.