5
\$\begingroup\$

I have been following along with a Udemy OOP basic course in Python. One of the sections prompted to make a banking system before watching the following videos. And so I have created a rudimentary banking system. I believe the system as is, is rather basic. I think there are some tweaks that can be made to it.

I have not use inheritance or any abstract base classes. Is there a possibility to utilize them in my solution? Are there ways to enhance the efficiency of my solution from an OOP point of standpoint?

My code continuously prompts the user for input on whether they are a new user or existing user. And does the following.

  • A new user is allowed to create an account with an initial deposit.
  • An existing user is allowed to either deposit, withdraw or display their account information.
  • A few basic error checks, with more to come. I have checked:
    • Whether a user with the same name and account number exists, if so create a new account number.
    • Check whether a user's account is there, if not enter correct details.

For future code:

  • Add in code so all transactions are saved and users can see a list of all deposits or withdrawals with dates.
  • More error checking.
  • A better menu system.
import pandas as pd
import os
import random
class Bank:

    def __init__(self, name, acc_num):
        self.name = name
        self.acc_num = acc_num

    def withdraw(self, amount):
        self.amount = amount
        self.column = name + "-" + str(self.acc_num)
        self.accounts_df = pd.read_excel(os.getcwd() + "/accounts.xlsx")
        self.accounts_df[self.column] = self.accounts_df[self.column].loc[0] - amount
        self.accounts_df.to_excel(os.getcwd() + "/accounts.xlsx", index=False)

    def deposit(self, amount):
        self.amount = amount
        self.column = name + "-" + str(self.acc_num)
        self.accounts_df = pd.read_excel(os.getcwd() + "/accounts.xlsx")
        self.accounts_df[self.column] = self.accounts_df[self.column].loc[0] + amount
        self.accounts_df.to_excel(os.getcwd() + "/accounts.xlsx", index=False)

    def display_balance(self):
        self.column = self.name + "-" + str(self.acc_num)
        self.accounts_df = pd.read_excel(os.getcwd() + "/accounts.xlsx")
        print(self.accounts_df[self.column].loc[0])


class User():

    def __init__(self, name):
        self.name = name

    def create_new_user(self, int_dep):
        self.int_dep = int_dep
        self.account_num = random.randint(10000, 99999)

        if not os.path.exists(os.getcwd() + "/accounts.xlsx"):
            data = {
                self.name + "-" + str(self.account_num): [self.int_dep]
            }
            self.int_dep_df = pd.DataFrame(data)
        else:
            self.int_dep_df = pd.read_excel(os.getcwd() + "/accounts.xlsx", index=False)
            print(self.account_num)
            print('bfor')
            while True:
                if self.name + "-" + str(self.account_num) in self.int_dep_df.columns:
                    self.account_num = random.randint(10000, 99999)
                else:
                    break

            self.int_dep_df[self.name + "-" + str(self.account_num)] = self.int_dep
        self.int_dep_df.to_excel(os.getcwd() + "/accounts.xlsx", index=False)
        return 'Your account has been created you can now login with your name: ' + self.name + ' and account number: '\
               + str(self.account_num)

    def existing_user(self, account_num):
        self.account_num = account_num
        ex_user_df = pd.read_excel(os.getcwd() + "/accounts.xlsx")
        if self.name + "-" + str(self.account_num) in ex_user_df.columns:
            return 'ok'
        else:
            return 'no user with these credentials, please enter again'


while True:
    print('enter 1 if existing user')
    print('enter 2 to new user')
    user_input = int(input())
    if user_input == 1:
        while True:
            name = input('Enter your name')
            acc_number = int(input('Enter your account number'))
            u = User(name)
            check_ex_user = u.existing_user(acc_number)
            if 'ok' in check_ex_user:
                while True:
                    print('enter 4 to deposit')
                    print('enter 5 to withdraw')
                    print('enter 6 to display')
                    print('enter 7 to exit')
                    bank_actions = int(input())
                    bank_ob = Bank(name, acc_number)
                    if bank_actions == 4:
                        print('Enter amount to deposit')
                        dep = int(input())
                        bank_ob.deposit(dep)
                    elif bank_actions == 5:
                        print('Enter amount to withdraw')
                        withdraw_am = int(input())
                        bank_ob.withdraw(withdraw_am)
                    elif bank_actions == 6:
                        print('Your current balance is;')
                        bank_ob.display_balance()
                    elif bank_actions == 7:
                        break
                break
            else:
                print(check_ex_user)
                continue
    elif user_input == 2:
        name = input('Enter your name')
        int_dep = int(input('Enter your initial deposit'))
        u = User(name)
        u.create_new_user(int_dep)
        continue
\$\endgroup\$
1
\$\begingroup\$

Your code uses menu technology from the 1980s. It is not user-friendly at all.

Instead of offering the user the choice of 1, 2, 3, 4, it is better to let the user choose the actions by naming them directly. As an example, see the following transcript, in which the lines starting with > show the user input:

Bank manager version 2020.1
Type 'help' to see the available commands.

> help
available commands:
help
register account <name> <balance>
delete account <account number>
list accounts
transfer <amount> from <source> to <target>
quit

> register account
usage: register account <name> <balance>

> register account "First Last" 12345.05
created account AC0001

> transfer 1234.44 from AC0001 to AC0007
error: target account AC0007 does not exist

> register account "Family" 0.00
created account AC0002

> transfer 1234.44 from AC0001 to AC0002
transferred

This kind of interaction is more natural to read than a sequence like 1, 3, 1, 5, 1, 1.

Of course it's a bit more work to split the lines into meaningful pieces, especially with the spaces in the name and the quotes around it, but it has the benefit that you can save all these interactions in a file and apply that file automatically, which is useful for testing your code after any changes.

Plus, the names of the actions are more stable than the menu item numbers. Whenever you change the structure of the menus, your users would have to re-learn the numbers and their associated actions.

Regarding your data model:

  • In normal life, a Bank is an institution that has many accounts. In your code, a bank is created whenever someone deposits or withdraws an amount. This wording is not correct. The bank should be a long-lived object. And it should be able to manage several accounts.

Regarding your code:

  • The variable bank_actions is wrong. The name must be bank_action since this variable only holds a single action.

  • In User.create_new_user you make the account number a random integer. That is wrong since account numbers must be unique. Having only 90000 different account numbers will quickly generate collisions. A bank must manage money and accounts more reliably. In particular, it must not reuse existing account numbers.

In summary, I suggest that you re-think the whole domain that you are modelling. Write down the possible actions in natural language and carefully check whether it makes sense to "deposit money to a bank", or whether it would be more appropriate to "deposit money to an account, managed by a bank". Be pedantic with each word, be as precise as possible.

And after that, you will probably have to rewrite your code so that it matches your new domain model again.

It's good to have an early prototype in code, but don't hesitate to throw it away. Usually the second attempt is much better than the first one.

\$\endgroup\$
3
  • \$\begingroup\$ This is really great info, and great detailing. I agree I think I have a problem with modelling projects like this in OOP, this is why I want to get better at, I guess I need to start thinking out the box a bit more, before I had just been working with data in pandas so a different type of modelling! \$\endgroup\$
    – mp252
    Jan 25 '20 at 23:32
  • \$\begingroup\$ Would you recommend for the menu code to create a class for that to? \$\endgroup\$
    – mp252
    Jan 25 '20 at 23:33
  • 1
    \$\begingroup\$ No, I would not use a class immediately. I would first do the menu code using functions. And if it becomes clear later that there are several functions that belong together and share some data, I would put them in a class. But before that, I prefer simple functions, just because they are simpler. \$\endgroup\$ Jan 26 '20 at 6:56

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.