Problem
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()
Solution
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 [].