Conception simple du système de banque-guichet OOP

Question

Pour m'entraîner à concevoir des systèmes, j'ai créé un système de guichet automatique bancaire simple.

Je ne voulais pas compliquer les choses, j'ai donc laissé de côté des choses comme la caisse par guichet automatique et l'ouverture de plateaux de caisse, etc. Je voulais aussi négliger les API pour valider les broches, etc. (c'est juste une fonction qui retournera toujours True) . Je voulais me concentrer sur les classes et les méthodes.

Donc dans mon exemple j'ai

• une Accountclasse qui a un numéro de compte, un solde et des fonctions de retrait et de dépôt.
• une Cardclasse qui a des comptes et une fonction de sélection de compte qui recherchera le numéro de compte sur la carte et le renverra.
• une ATMclasse qui a les variables suivantes: card_inserted, card_validated, current_account, current_cardet la fonction principale sont perform_request( ce qui donnera soit le solde, le dépôt ou retirer de l' argent), validate_pin(qui fera l'appel à l'API, moqué dans mon code), select_account(qui sélectionnez un compte sur la carte)

J'ai écrit un test pour cela et cela a fonctionné. Je me demandais si je pourrais avoir des commentaires à ce sujet (pas le test, je sais que je peux faire beaucoup de choses pour améliorer les tests, mais ce n'était qu'une version rapide et sale de celui-ci)?

atm.py:

def validate_api(card_nr, pin):
    # validate account nr and pin
    # return true or fale
    return True

class Card():
    def __init__(self, accounts = []):
        self.accounts = accounts

    def select_account(self, account_nr):
        # check if there is a built in filter func in python

        for account in self.accounts:
            if account.account_nr == account_nr:
                return account

        raise Exception("Account number invalid")

class Account():
    def __init__(self, account_nr = None):
        self.account_nr = account_nr
        self.balance = 0

    def deposit(self, amount):
        self.balance += amount

    def withdraw(self, amount):
        self.balance -= amount


class ATM():
    def __init__(self):
        self.card_inserted = False
        self.card_validated = False
        self.current_account = None
        self.current_card = None

    def validate_card_and_pin(self):
        if not self.card_inserted:
            raise Exception('Card must be inserted first')

        if self.card_validated is False:
            raise Exception
            # raise Exception("Card not validated")

    def insert_card(self, bank_card):
        # code to accept card
        self.card_inserted = True
        self.current_card = bank_card

    def eject_card(self):
        self.end_session()
        # code to push card out

    def validate_pin(self, pin):
        if not self.card_inserted:
            raise Exception('Card must be inserted first')

        # card is inserted, pin has to be validated
        # post pin and card number to api
        # response will be saved in validated variable

        validated = validate_api(card_nr=0,pin=0)

        if validated == False:
            self.card_validated = False
            return self.card_validated

        self.card_validated = True
        return self.card_validated

    def select_account(self, account_nr):
        self.validate_card_and_pin()
        if self.current_card is None:
            raise Exception("no card in ATM")

        if self.card_validated is False:
            raise Exception("card not validated")

        current_account = self.current_card.select_account(account_nr)
        self.current_account = current_account


    '''
    1 = check balance
    2 = deposit money
    3 = withdraw money
    '''
    def perform_request(self, request, amount = 0):
        self.validate_card_and_pin()
        if request == 1:
            return self.check_balance()
        elif request == 2:
            return self.accept_cash(amount)
        elif request == 3:
            return self.give_out_cash(amount)
        else:
            raise Exception("invalid request")

    def accept_cash(self, amount):
        # open cash tray
        # count cash
        self.current_account.deposit(amount)
        return amount

    def give_out_cash(self, amount):
        # count cash
        # open tray
        self.current_account.withdraw(amount)
        return amount

    def check_balance(self):
        return self.current_account.balance

    def end_session(self):
        self.card_inserted = False
        self.card_validated = False
        self.current_account = None
        self.current_card = None
        return True

Et voici le test:

 def test_depositing_50_and_withdrawing_20():
        checking = Account(1)
        saving = Account(2)
        bank_card = Card([checking, saving])
        atm = ATM()
    
        atm.insert_card(bank_card)
        atm.validate_pin("123")
        atm.select_account(1)
        atm.perform_request(2, 50)
        if atm.perform_request(1) is not 50:
            raise Exception("depositing function doesn't work")
        atm.end_session()
    
        atm.insert_card(bank_card)
        atm.validate_pin("123")
        atm.select_account(1)
        atm.perform_request(3, 20)
        if atm.perform_request(1) is not 30:
            raise Exception("withdrawing function doesn't work")
    
        atm.select_account(2)
        atm.validate_pin("123")
        atm.perform_request(2, 10)
    
        if atm.perform_request(1) is not 10:
            raise Exception("depositing function doesn't work")
    
        print("Test successful") 

Answer 1

class Card():

Vous n'avez pas besoin de parenthèses dans les déclarations de classe. De plus, bien que la plupart de votre formatage soit plutôt bon, il y a quelques points délicats. Lors de la fourniture d'un argument par défaut, il ne doit pas y avoir d'espaces autour du= , et il doit y avoir deux lignes vides entre les définitions de niveau supérieur .

---

Vous ne devriez vraiment jamais avoir de paramètre par défaut mutable comme vous le faites dans:

def __init__(self, accounts = []):

Dans ce cas, puisque vous ne modifiez jamais self.accounts, vous êtes en sécurité. Si vous avez déjà ajouté une add_accountméthode qui associe un compte avec une carte par appendING self.accounts, vous verriez que tout Carddans votre programme qui a été créée en utilisant l'argument par défaut changerait lorsque cette méthode est exécutée.

Je le changerais en:

def __init__(self, accounts=None):  # Default to None
    self.accounts = accounts or []  #  then create the list inside the method instead

---

# check if there is a built in filter func in python

Il existe plusieurs façons d'utiliser une fonction de raccourci sophistiquée pour trouver le premier élément qui satisfait un prédicat, mais honnêtement, je m'en tiendrai à ce que vous avez. Une forboucle avec un retour anticipé est assez facile à comprendre. Puisque vous souhaitez lever une exception si un compte n'est pas trouvé, les autres méthodes deviennent un peu bâclées. Si cela vous convenait de le faire revenir Nonesur une erreur, vous pouvez utiliser:

def select_account(self, account_nr):
    return next((account for account in self.accounts if account.account_nr == account_nr), None)

nexttente de saisir le premier élément d'un itérateur que vous lui donnez. Je lui donne une expression de générateur, qui ne produira un élément que si account.account_nr == account_nrc'est vrai. L' Noneas le deuxième argument est une valeur par défaut si rien n'est trouvé. forCependant, je préfère toujours le style itératif .

---

Dans Account, vous autorisez Noneà être utilisé comme numéro de compte. Cela me semble être le genre de champ qui ne devrait pas être "nullable", ou être autorisé à être omis lors de la création d'un objet. Je pense qu'un numéro de compte (unique et existant) est assez fondamental pour l'idée d'un compte bancaire. Je supprimerais la valeur par défaut et forcerais l'utilisateur à fournir ces informations lors de la création d'un compte. Cependant, il peut être utile d'avoir un deuxième starting_balanceparamètre qui permet à l'utilisateur de définir le solde du compte de départ et de l'autoriser par défaut 0.

---

validate_card_and_pinest un nom trompeur. Cela ne semble pas faire les validations. Il s'attend à ce que les validations aient déjà été effectuées et que l' self.card_validatedétat interne soit déjà défini. assert_is_validatedpeut être un meilleur nom pour ce qu'il fait.

---

Je pense que cela validate_pinpourrait être plus clair. Notez comment une grande partie du code en bas est dupliquée. Vous définissez card_validatedce qui validatedest, puis renvoyez cette valeur. La fonction peut être simplement:

def validate_pin(self, pin):
    if not self.card_inserted:
        raise Exception('Card must be inserted first')
    
    self.card_validated = validate_api(card_nr=0,pin=0)
    return self.card_validated

---

Je repenserais peut-être jeter des exceptions aux méthodes. À mes yeux, un code PIN mal saisi par exemple n'est pas vraiment exceptionnel . Je suis fan de revenir en Nonetant qu'indicateur d'erreur dans les cas où une fonction ne peut échouer que d'une manière; comme validate_pin. Vous devez juste avoir l'habitude d'identifier le retour d'une fonction Noneet de gérer correctement ce cas.

Cependant, si vous souhaitez utiliser des exceptions, lancer une plaine Exceptionest une mauvaise idée. Cela rend plus difficile pour l'appelant de détecter et de gérer spécifiquement les exceptions de votre code. Je pense que c'est un cas approprié pour créer des exceptions personnalisées . Quelque chose comme:

class PINValidationFailed(RuntimeError):
    pass

Ensuite, l'appelant peut intercepter spécifiquement cette exception pour gérer les échecs de code PIN.

---

'''
1 = check balance
2 = deposit money
3 = withdraw money
'''
def perform_request(self, request, amount = 0):

Si cela est conçu comme une docstring, il doit être à l'intérieur de la fonction et indenté:

def perform_request(self, request, amount=0):
    '''
    1 = check balance
    2 = deposit money
    3 = withdraw money
    '''

---

Je pense que if self.card_validated is False:c'est plus clair aussi simplement if not self.card_validated:.

---

Après avoir retouché ceci, il me reste:

def validate_api(card_nr, pin):
    # validate account nr and pin
    # return true or false
    return True


class Card:
    def __init__(self, accounts=None):
        self.accounts = accounts or []

    def select_account(self, account_nr):
        return next((account for account in self.accounts if account.account_nr == account_nr), None)


class Account:
    def __init__(self, account_nr, starting_balance=0):
        self.account_nr = account_nr
        self.balance = starting_balance

    def deposit(self, amount):
        self.balance += amount

    def withdraw(self, amount):
        self.balance -= amount


class ATM:
    def __init__(self):
        self.card_inserted = False
        self.card_validated = False
        self.current_account = None
        self.current_card = None

    def validate_card_and_pin(self):
        if not self.card_inserted:
            raise RuntimeError('Card must be inserted first')

        if not self.card_validated:
            raise RuntimeError("Card not validated")

    def insert_card(self, bank_card):
        # code to accept card
        self.card_inserted = True
        self.current_card = bank_card

    def eject_card(self):
        self.end_session()
        # code to push card out

    def validate_pin(self, pin):
        if not self.card_inserted:
            raise RuntimeError('Card must be inserted first')

        self.card_validated = validate_api(card_nr=0, pin=0)
        return self.card_validated


    def select_account(self, account_nr):
        self.validate_card_and_pin()
        if self.current_card is None:
            raise RuntimeError("no card in ATM")

        if self.card_validated is False:
            raise RuntimeError("card not validated")

        current_account = self.current_card.select_account(account_nr)
        self.current_account = current_account


    def perform_request(self, request, amount=0):
        '''
        1 = check balance
        2 = deposit money
        3 = withdraw money
        '''
        self.validate_card_and_pin()
        if request == 1:
            return self.check_balance()
        elif request == 2:
            return self.accept_cash(amount)
        elif request == 3:
            return self.give_out_cash(amount)
        else:
            raise RuntimeError("invalid request")

    def accept_cash(self, amount):
        # open cash tray
        # count cash
        self.current_account.deposit(amount)
        return amount

    def give_out_cash(self, amount):
        # count cash
        # open tray
        self.current_account.withdraw(amount)
        return amount

    def check_balance(self):
        return self.current_account.balance

    def end_session(self):
        self.card_inserted = False
        self.card_validated = False
        self.current_account = None
        self.current_card = None
        return True