5
\$\begingroup\$

Would my code be considered fairly optimal and efficient? If not, where can I look to improve the code? The program works just fine.

Would this method scale well if the dictionary containing the number of bank account increases?

Design: I have designed and kept only one object in main(). i.e. bank_1, contains a dictionary of information regarding multiple bank accounts. I have elected to only create the object bank_account inopen_bank_account() and check_bank_blanance() when needed and these objects disappear once the function is done.

Intention: The intention here is to use as few objects as possible in main().

Output: The program runs just fine; just enter 1, then enter 2, then enter 777 followed by 777. It will print the bank account balance of $1.00 of the account id 777.

class Bank_Account:
    def __init__(self, account_id, account_pin, account_balance):
        self._account_id = account_id
        self._account_pin = account_pin
        self._account_balance = account_balance
        self._individual_bank_account_dicitionary = {}
        self._individual_bank_account_dicitionary[account_id] = {"Pin": account_pin, "Balance": account_balance}

    def get_bank_account_balance(self, account_id):
        return "Balance: ${:.2f}".format(self._individual_bank_account_dicitionary[account_id]["Balance"])

    def __str__(self):
        return "{}".format(self._individual_bank_account_dicitionary)

class Bank:
    def __init__(self, bank_name):
        self._bank_name = bank_name
        self._bank_dicitionary = {}

    def update_bank_dictionary(self, bank_account):
        self._bank_dicitionary[bank_account._account_id] = {"Pin": bank_account._account_pin, "Balance": bank_account._account_balance}

    # 1. A method to return the dictionary of bank accounts of the object from class Bank.
    # 2. In this case, only bank_1 from main() is the object argument.      
    def get_bank_dictionary(self):
        return self._bank_dicitionary

    # 1. Method to create object bank_account only when needed.
    def get_bank_account(self, account_id):
        account_pin = self._bank_dicitionary[account_id]["Pin"]
        account_balance = self._bank_dicitionary[account_id]["Balance"]
        bank_account = Bank_Account(account_id, account_pin, account_balance)
        return bank_account

    # 1. This is used to convert the dictionary into a string for printing purposes.
    def string_bank_dictionary(self):
        string_list = ["account ID: {} \naccount Pin: {} \nBank Balance: {} \n".format(key, self._bank_dicitionary[key]["Pin"], self._bank_dicitionary[key]["Balance"])\
                      for key, value in self._bank_dicitionary.items()]
        return "\n".join(string_list) 

    def __str__(self):
        return "{}".format(self.string_bank_dictionary())

def open_bank_account(bank):
#     # Uncomment out when running actual program.
#     account_id = input("Enter account id here: ")
#     account_pin = input("Enter account pin here: ")
#     account_balance = input("Enter account initial balance here: ")

    # Comment out when running actual program. 
    # Currently in used for testing purposes.
    account_id = 455
    account_pin = 123
    account_balance = 888
    bank_account = Bank_Account(account_id, account_pin, account_balance)
    bank.update_bank_dictionary(bank_account)

    # Comment out when running actual program.
    # Currently in used for testing purposes.
    account_id = 777
    account_pin = 777
    account_balance = 1
    bank_account = Bank_Account(account_id, account_pin, account_balance)
    bank.update_bank_dictionary(bank_account)

    # Comment out when running actual program. 
    # Currently in used for testing purposes.   
    account_id = 631
    account_pin = 222
    account_balance = 50
    bank_account = Bank_Account(account_id, account_pin, account_balance)
    bank.update_bank_dictionary(bank_account)

    return bank

def check_bank_blanance(bank):
    valid_id_password = False
    temporary_dictionary = bank.get_bank_dictionary()
    while True:
        account_id = int(input("Enter account id here: "))
        account_pin = int(input("Enter account pin here: "))
        for key in temporary_dictionary.keys():
            if account_id == key and temporary_dictionary[account_id]["Pin"] == account_pin:
                valid_id_password = True
                bank_account = bank.get_bank_account(account_id)
                print(bank_account.get_bank_account_balance(account_id))
                break
        if valid_id_password == True:
            break
        else:
            print("Invalid account id/password. Please try again.")

def main():
    bank_1 = Bank("ABC Bank")
    while True:
        print("Menu \n1. Open bank account \n2. Check balance")
        while True:
            account_choice = int(input("Enter option here: "))
            if account_choice <= 0 and account_choice >= 7:
                account_choice = int(input("Enter option here: "))
            else:
                break        
        if account_choice == 6:
            break
        elif account_choice == 1:
            bank_1 = open_bank_account(bank_1)
        elif account_choice == 2:
            balance = check_bank_blanance(bank_1)

main()
\$\endgroup\$
1
\$\begingroup\$

Would this method scale well if the dictionary containing the number of bank account increases?

Yes, I think so - dictionary access is fast, O(1) or "constant time".

Typo

check_bank_blanance should be check_bank_balance, or more likely check_balance.

_individual_bank_account_dicitionary should be _individual_bank_account_dictionary. That said - why does this dict exist at all? From what I see it only contains attributes that should be class members.

Method names

Just call get_bank_account_balance get_balance - or, if you make it a @property, simply balance.

str

return "{}".format(self._individual_bank_account_dicitionary)

should just be

return str(self._individual_bank_account_dictionary)

Class names

Bank_Account should just be named BankAccount.

Data exposure

The method get_bank_dictionary probably shouldn't exist. You're exposing data that should be dealt with by the class itself. This code:

    for key in temporary_dictionary.keys():
        if account_id == key and temporary_dictionary[account_id]["Pin"] == account_pin:

should exist in the Bank class, perhaps as get_account(). Also, don't loop through the dict to find a key - just do a dict lookup using [].

| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.