5
\$\begingroup\$

Python beginner here. Building an OOP bank account program with SQLite and starting to struggle a bit with its design. From a design standpoint I have a file (bank) which calls ATM. ATM simulates an ATM, and is the file which then calls bank_account or credit_card depending on the account data passed in from bank.

To initially setup an account I decided to put this into a different file, for example bank_account_setup or for credit_card, credit_card_setup. These would create_account, help setup pin etc so that the account is created and ready to use. Then the actual bank_account or credit_card contains other functions, like, deposit, withdraw, get_balance etc. Also, send_email is in another file

My question is basically around my design. Is there a way to structure this better? How about my setup files to create bank or credit card accounts? Is that a good or bad idea? Also, another issue I am having is that when I run bank I pass in account type to ATM. In ATM I then had to know what class I am using in advance and instantiate that inside ATM. Could I handle that dynamically? (Also, the code does work - just concerned with bad design).

DB Schema (this is mostly correct but some fields may have been added by hand to sqlite):

c.execute("""CREATE TABLE IF NOT EXISTS bank_account  (

        name text,
        social integer,
        account_number integer PRIMARY KEY,
        balance integer,
        pin integer
        )""")

c.execute("""CREATE TABLE IF NOT EXISTS credit_card (

        name text,
        social integer,
        account_number integer PRIMARY KEY,
        balance integer,
        card_no integer,
        credit_score integer,
        credit_limit integer
        )""")

c.execute("""CREATE TABLE IF NOT EXISTS savings_account (

        name text,
        social integer,
        account_number integer PRIMARY KEY,
        balance integer,
        rate real
        )""")

c.execute("""CREATE TABLE IF NOT EXISTS notifications (

        name text,
        email_address,
        account_number integer PRIMARY KEY,
        account_type,
        notif_bal,
        notif_deposits,
        notif_overdraft
        )""")

c.execute("""CREATE TABLE IF NOT EXISTS auth_code (

        account_number integer PRIMARY KEY,
        account_type,
        email,
        auth_code
        )""")

Here's my calling file bank:

import atm

class BankAccount:

    def __init__(self, name, social, account_number, balance, acctype):
        self.name = name
        self.social = social
        self.account_number = account_number
        self.balance = balance
        self.acctype = acctype

if __name__ == '__main__':

    obj1 = atm.ATM.main_menu( "Frank Smith", 135063522, 5544, 850, 'credit_card', 4400110022004400)

Here's ATM, which calls the other files:

import sqlite3, smtplib
import bank_account
import secrets
import send_email
import credit_card
from email.mime.text import MIMEText
from email.mime.multipart import MIMEMultipart
from email.mime.base import MIMEBase
from email import encoders

conn = sqlite3.connect('bank_account.db')
c = conn.cursor()

class ATM:

    def get_pin(account_number, acctype, user_pin):
        with conn:
            db_pin =  c.execute("SELECT pin from {} WHERE account_number=:account_number".format(acctype),
                      {'account_number':account_number})
            db_pin = c.fetchone()
            if db_pin is not None:
                return(db_pin[0])
            else:
                print("db pin is None")
                pass

    def set_pin(account_number, acctype, input_code):
        with conn:
            get_code = ATM.get_user_code(account_number, acctype)
            if get_code is None:
                pass
                print("You need to request an authorization code first before you set your pin")
            else:    
                if get_code !=input_code:
                    print("Authorization code not valid")
                else:    
                    pin = input("Please set your 4 digit pin: ")
                    if len(pin) < 4 or len(pin) >4 or len(pin) == 4 and pin.isdigit()==False:
                        print("This is not a 4 digit pin")
                    else:
                        print("pin accepted")
                        c.execute("""UPDATE {} SET pin=:pin
                                WHERE account_number =:account_number""".format(acctype),
                                  {'account_number':account_number, 'pin':pin})
                        print("Pin for account has been updated")


    def main_menu(name, social, account_number, balance, acctype, card_no=None):

#        obj1 =  bank_account.BankAccount(name, social, account_number, balance, acctype)    
#       obj1 =  credit_card.CreditCard(name, social, account_number, balance, acctype, card_no)         
        user_pin = int(input("\nATM Home Screen. Please enter your pin code: "))
        db_pin = ATM.get_pin(account_number, acctype, user_pin)
        if user_pin != db_pin and db_pin != '':
            print("No pin match")
        elif db_pin is '':
            print("Pin has not been set")
            print("First request an authorization code and use that to set the pin")
        else:
            user_pin == db_pin
            print("\nPin accepted continue \n ")
            print("""""""ATM Menu, choose an option""""""")  
            print("\n1 - Deposit funds")
            print("2 - Withdraw funds")
            print("3 - Check balance")
            print("4 - Reset Pin")
            print("5 - Exit")

            while True:
                try:
                    choice = int(input("Please enter a number: "))
                except ValueError:
                    print("This is not a number")
                if choice >= 1 and choice <=5:
                    if choice == 1:
                        amount = input("\nPlease enter the deposit amount: ")
                        if amount != '' and amount.isdigit():
                            int(amount)
                            obj1.deposit( account_number, acctype, amount)
                        else:
                            print("Please enter a valid number")

                    elif choice == 2:
                        amount = input("Please enter the withdrawl amount: ")
                        if amount != '' and amount.isdigit():
                            int(amount)
                            obj1.withdraw(account_number, acctype, amount)
                        else:
                            print("Please enter a valid number")

                    elif choice ==3:
                        obj1.get_balance(account_number, acctype)

                    elif choice ==4:
                        new_pin = input("Please enter a new 4 digit pin: ")
                        if new_pin != '' and new_pin.isdigit():
                            int(new_pin)
                        obj1.set_reset_pin(account_number, acctype, new_pin)

                    elif choice ==5:
                        break

            else:
                print("Not a valid number")

ALL of bank_account_setup:

import sqlite3
import secrets
import getpass
import smtplib, sqlite3
from email.mime.text import MIMEText
from email.mime.multipart import MIMEMultipart
from email.mime.base import MIMEBase
from email import encoders
import send_email

conn = sqlite3.connect('bank_account.db')
c = conn.cursor()        

class BankAccount:

    def __init__(self, name, social, account_number, balance, acctype):
        self.name = name
        self.social = social
        self.account_number = account_number
        self.balance = balance
        self.acctype = acctype

    """ create different accounts based on account type passed in """         
    def create_account(self, name, social, account_number, balance, acctype, card_no=None, credit_score=None, credit_limit=None):
        self.rate = None
        with conn:
        #  account_found = BankAccount.get_account(self, account_number, acctype)   
        #   if not account_found:
            if acctype == 'bank_account':
                c.execute("INSERT INTO {} VALUES (:name, :social, :account_number, :balance, :pin)".format(acctype),
                          {'name':name, 'social': social,'account_number': account_number, 'balance':balance, 'pin':''})  
                print("New account: {} has been created, acc # is: {}".format(acctype, account_number))

            elif acctype == 'savings_account':
                c.execute("INSERT INTO {} VALUES (:name, :social, :account_number, :balance, :rate)".format(acctype),
                      {'name':name, 'social': social,'account_number': account_number, 'balance':balance, 'rate':''})
                print("New account: {} has been created, acc # is: {}".format(acctype, account_number))

            elif acctype == 'credit_card':
                c.execute("INSERT INTO credit_card VALUES (:name, :social, :account_number, :balance, :card_no,:credit_score, :credit_limit, :pin)",
                  {'name':name, 'social': social,'account_number': account_number, 'balance':balance, 'card_no'
                   :card_no, 'credit_score':credit_score, 'credit_limit':credit_limit, 'pin':'' })
                print("New account: {} has been created, acc # is: {}".format(acctype, account_number))                                  
        conn.commit()

    """ Show all rows in DB for the the account type passed in """
    def get_account(self,account_number, acctype):
        with conn:
            account_find = c.execute("SELECT * from {} WHERE account_number=:account_number".format(acctype),
                                     {'account_number':account_number})
            account_found = c.fetchone()
            if not account_found:
                print("No {} matching that number could be found".format(acctype))
            else:
                print("Account type: {} exists!".format(acctype))
                print(account_found)
        return(account_found)    

    """ Generate a random string for card activation """
    def set_user_code(self, account_number, acctype, email):
        with conn:
        #  account_found = BankAccount.get_account(self, account_number, acctype)
            account_found = BankAccount.get_user_code(self, account_number, acctype)
            if not account_found:
                auth_code = secrets.token_hex(4)
                print("User code {} generated".format(auth_code))
                c.execute("INSERT INTO auth_code VALUES (:account_number, :acctype, :email, :auth_code)",
                               {'account_number': account_number, 'acctype': acctype, 'email':email, 'auth_code':auth_code})
                print("DB updated with auth code for account")

                subject = 'Authorization code'
                body = 'Authorization code: {}\n \
                Use the authorization code when setting your pin for the first time'.format(auth_code)
                email_user = '[email protected]'
                email_send = '[email protected]'
                email_pass = 'Liverpool27'

                msg = MIMEMultipart()
                msg['From'] = email_user
                msg['To'] = email_send
                msg['Subject'] = subject

                msg.attach(MIMEText(body, 'plain'))
                server = smtplib.SMTP('smtp.gmail.com: 587')
                server.starttls()
                server.login(email_user, email_pass)
                text = msg.as_string()
                server.sendmail(email_user, email_send, text)
                server.quit               
            else:
                print("Auth code {} is already set for this account {} ".format(account_found[2], account_number))
        conn.commit()


    def get_user_code(self,account_number, acctype):
        with conn:
            account_found = c.execute("SELECT auth_code from auth_code WHERE account_number=:account_number",
            {'account_number':account_number})
            account_found = c.fetchone()
        if account_found is None:
            pass
            #   print("You need to request an authorization code first before you set your pin")
        else:    
            return(account_found[0])

    """ Set pin for an account based on the auth code entered for validation """
    def set_pin(self,account_number, acctype, input_code):
        with conn:
            get_code = BankAccount.get_user_code(self, account_number, acctype)
            if get_code is None:
                pass
                print("You need to request an authorization code first before you set your pin")
            else:    
                if get_code !=input_code:
                    print("Authorization code not valid")
                else:    
                    pin = input("Please set your 4 digit pin: ")
                    if len(pin) < 4 or len(pin) >4 or len(pin) == 4 and pin.isdigit()==False:
                        print("This is not a 4 digit pin")
                    else:
                        print("pin accepted")
                        c.execute("""UPDATE {} SET pin=:pin
                                WHERE account_number =:account_number""".format(acctype),
                                  {'account_number':account_number, 'pin':pin})
                        print("Pin for account has been updated")



if __name__ == '__main__':
    obj1 = BankAccount("Alexis Sanchez", 135063522, 5534, 100, 'bank_account')
#    obj1.create_account("Alexis Sanchez", 135063522, 5534, 100, 'bank_account')
#    obj1.set_user_code(5534, 'bank_account', '[email protected]')
    obj1.set_pin(5534, 'bank_account', '1f7bd3f9')

ALL of bank_account:

import sqlite3
import secrets
import getpass
import smtplib, sqlite3
from email.mime.text import MIMEText
from email.mime.multipart import MIMEMultipart
from email.mime.base import MIMEBase
from email import encoders
import send_email

conn = sqlite3.connect('bank_account.db')
c = conn.cursor()        

class BankAccount:

    def __init__(self, name, social, account_number, balance, acctype):
        self.name = name
        self.social = social
        self.account_number = account_number
        self.balance = balance
        self.acctype = acctype

    """ create different accounts based on account type passed in """         
    def create_account(self, name, social, account_number, balance, acctype, card_no=None, credit_score=None, credit_limit=None):
        self.rate = None
        with conn:
        # account_found = BankAccount.get_account(self, account_number, acctype)   
        # if not account_found:
            if acctype == 'bank_account':
                c.execute("INSERT INTO {} VALUES (:name, :social, :account_number, :balance, :pin)".format(acctype),
                          {'name':name, 'social': social,'account_number': account_number, 'balance':balance, 'pin':''})  
                print("New account: {} has been created, acc # is: {}".format(acctype, account_number))

            elif acctype == 'savings_account':
                print("Savings account")
                c.execute("INSERT INTO {} VALUES (:name, :social, :account_number, :balance, :rate)".format(acctype),
                      {'name':name, 'social': social,'account_number': account_number, 'balance':balance, 'rate':''})
                print("New account: {} has been created, acc # is: {}".format(acctype, account_number))

            elif acctype == 'credit_card':
                c.execute("INSERT INTO credit_card VALUES (:name, :social, :account_number, :balance, :card_no,:credit_score, :credit_limit, :pin)",
                  {'name':name, 'social': social,'account_number': account_number, 'balance':balance, 'card_no'
                   :card_no, 'credit_score':credit_score, 'credit_limit':credit_limit, 'pin':'' })
                print("New account: {} has been created, acc # is: {}".format(acctype, account_number))                                  
        conn.commit()

    """ Generate a random string for card activation """
    def set_user_code(self, account_number, acctype, email):
        with conn:
        #  account_found = BankAccount.get_account(self, account_number, acctype)
            account_found = BankAccount.get_user_code(self, account_number, acctype)
            if not account_found:
                auth_code = secrets.token_hex(4)
                print("User code {} generated".format(auth_code))
                c.execute("INSERT INTO auth_code VALUES (:account_number, :acctype, :email, :auth_code)",
                               {'account_number': account_number, 'acctype': acctype, 'email':email, 'auth_code':auth_code})
                print("DB updated with auth code for account")

                subject = 'Authorization code'
                body = 'Authorization code: {}\n \
                Use the authorization code when setting your pin for the first time'.format(auth_code)
                email_user = '[email protected]'
                email_send = '[email protected]'
                email_pass = 'Liverpool27'

                msg = MIMEMultipart()
                msg['From'] = email_user
                msg['To'] = email_send
                msg['Subject'] = subject

                msg.attach(MIMEText(body, 'plain'))
                server = smtplib.SMTP('smtp.gmail.com: 587')
                server.starttls()
                server.login(email_user, email_pass)
                text = msg.as_string()
                server.sendmail(email_user, email_send, text)
                server.quit               
            else:
                print("Auth code {} is already set for this account {} ".format(account_found[2], account_number))
        conn.commit()


    def get_user_code(self,account_number, acctype):
        with conn:
            account_found = c.execute("SELECT auth_code from auth_code WHERE account_number=:account_number",
            {'account_number':account_number})
            account_found = c.fetchone()
        if account_found is None:
            pass
        # print("You need to request an authorization code first before you set your pin")
        else:    
            return(account_found[0])

    """ Set pin for an account based on the auth code entered for validation """
    def set_pin(self,account_number, acctype, input_code):
        with conn:
            get_code = BankAccount.get_user_code(self, account_number, acctype)
            if get_code is None:
                pass
                print("You need to request an authorization code first before you set your pin")
            else:    
                if get_code !=input_code:
                    print("Authorization code not valid")
                else:    
                    pin = input("Please set your 4 digit pin: ")
                    if len(pin) < 4 or len(pin) >4 or len(pin) == 4 and pin.isdigit()==False:
                        print("This is not a 4 digit pin")
                    else:
                        print("pin accepted")
                        c.execute("""UPDATE {} SET pin=:pin
                                WHERE account_number =:account_number""".format(acctype),
                                  {'account_number':account_number, 'pin':pin})
                        print("Pin for account has been updated")

    """ Reset pin, pass in new pin """
    def set_reset_pin(self, account_number, acctype, new_pin):
        with conn:
            c.execute("""UPDATE {} SET pin=:new_pin
                                WHERE account_number =:account_number""".format(acctype),
                                  {'account_number':account_number, 'new_pin':new_pin})
            print("Pin for account has been updated")
            conn.commit()            


    def get_pin(self, account_number, acctype, user_pin):
        with conn:
            db_pin =  c.execute("SELECT pin from {} WHERE account_number=:account_number".format(acctype),
                      {'account_number':account_number})
            db_pin = c.fetchone()
            if db_pin is not None:
                return(db_pin[0])
            else:
                print("db pin is None")
                pass


    """ Set email notification preferences for users who have confirmed bank accounts """
    """ Do an insert if no notifications record is found """
    """ If there is a notifications record, update all notifications for that user """
    def set_notifications(self, name, email_address, account_number, acctype, notif_bal, notif_deposits, notif_overdraft, notif_withdraw):
        with conn:
            """ check if an account is found first """
            account_found = BankAccount.get_account(self, account_number, acctype)

            """ Check if a notification record is found """
            notif_found = BankAccount.get_notif(self, account_number, acctype)


            if account_found and notif_found is None:
                c.execute("""INSERT INTO notifications VALUES (:name, :email_address, :account_number, :acctype, :notif_bal, :notif_deposits, :notif_overdraft, :notif_withdraw)""".format(acctype),
                                  {'name':name, 'email_address': email_address,'account_number': account_number, 'acctype':acctype, 'notif_bal':notif_bal,
                                   'notif_deposits':notif_deposits, 'notif_overdraft':notif_overdraft, 'notif_withdraw':notif_withdraw})  
                print("Notifications for acc#{} has been created, acctype {} have been setup".format(account_number, acctype))


            elif account_found and notif_found:
                c.execute("""UPDATE notifications SET notif_bal=:notif_bal, notif_deposits=:notif_deposits, notif_overdraft=:notif_overdraft, notif_withdraw=:notif_withdraw""",
                          {'notif_bal':notif_bal, 'notif_deposits':notif_deposits, 'notif_overdraft':notif_overdraft, 'notif_withdraw': notif_withdraw})
                print("Notificatons for acc# {} have been updated".format(account_number))
            else:
                print("Setup an account first then set notifications")

    def get_notif(self,account_number, acctype):
        with conn:
            notif_exist = c.execute("""SELECT * from notifications WHERE account_number=:account_number and
                                      account_type=:account_type""",
                                         {'account_number':account_number, 'account_type':acctype})
            notif_exist = c.fetchone()
        return(notif_exist)


    """ Show all rows in DB for the the account type passed in """
    def get_account(self,account_number, acctype):
        with conn:
            account_find = c.execute("SELECT * from {} WHERE account_number=:account_number".format(acctype),
                                     {'account_number':account_number})
            account_found = c.fetchone()
            if not account_found:
                print("No {} matching that number could be found".format(acctype))
            else:
                print("Account type: {} exists!".format(acctype))
                print(account_found)
        return(account_found)

    def get_balance(self, account_number, acctype):
        with conn:
            balance = c.execute("SELECT balance from {} WHERE account_number=:account_number".format(acctype),
                                {'account_number':account_number})
            balance = c.fetchone()
            print("The balance for account number: {} is ${}".format(account_number, balance[0]))
            notif_set = BankAccount.get_notif(self, account_number, acctype)
            if notif_set is None:
                print("No notifications are set for this user")
            else:    
                notif_balance = notif_set[4]
                name = notif_set[0]
                if notif_balance == 1:
                    notify = send_email.send_email(account_number, acctype, 'Balance', balance, balance, name)

        return(balance[0])

    """ Deposit funds into the account number + acctype for the account passed in """
    def deposit(self, account_number, acctype, amount):
        with conn:

            """ Check acct exists before making deposit """
            account_found = BankAccount.get_account(self, account_number, acctype)
            if account_found:
                existing_bal = account_found[3]
                c.execute("""UPDATE {} SET balance=balance +:amount
                        WHERE account_number =:account_number""".format(acctype),
                          {'account_number':account_number, 'amount':amount})
                new_bal = existing_bal + (int(amount))
                print("${} has been deposited to account {} and the new balance is ${}".format(amount, account_number, existing_bal + (int(amount))))

               # Check email configurations are turned on for deposits 
                notif_set = BankAccount.get_notif(self, account_number, acctype)
                if notif_set is None:
                    print("No notifications are set for this user")
                else:    
                    notif_deposits = notif_set[5]
                    name = notif_set[0]
                    if notif_deposits == 1:
                        notify = send_email.send_email(account_number, acctype, 'Deposit', amount, new_bal, name)

    """ withdraw funds from the bank account passed in """       
    def withdraw(self, account_number, acctype, amount):
        with conn:
            """ Check account exists """
            account_found = BankAccount.get_account(self, account_number, acctype)
            existing_bal = account_found[3]

            if account_found:  
                c.execute("""UPDATE bank_account SET balance=balance -:amount
                        WHERE account_number =:account_number""",
                          {'account_number':account_number, 'amount':amount})
                new_bal = existing_bal - (int(amount))
                conn.commit() 
                print("${} has been withdrawn from account {} and the new balance is ${}".format(amount, account_number, existing_bal - (int(amount))))

                notif_set = BankAccount.get_notif(self, account_number, acctype)
                if notif_set is None:
                    print("No notifications have been set for this acct")
                else:
                    notif_withdraw = notif_set[7]
                    name = notif_set[0]
                    if notif_withdraw == 1:
                        notify = send_email.send_email(account_number, acctype, 'Withdraw', amount, new_bal, name)
            else:
                print("Withdrawl notifications have been turned off")
            if account_found and new_bal < 0 and notif_set is not None:
                notify_o = send_email.send_email(account_number, acctype, 'Overdraft', amount, new_bal, name)    
            conn.commit()

ALL of credit card setup:

from bank_account import BankAccount
import sqlite3

conn = sqlite3.connect('bank_account.db')
c = conn.cursor()     

class CreditCard(BankAccount):
    def __init__(self, name, social, account_number, balance, acctype, card_no, credit_score=None, credit_limit=None):
        super().__init__(name, social, account_number, balance, acctype)
        self.card_no = card_no
        self.credit_score = credit_score
        self.credit_limit = credit_limit

    """ set credit limit, check if acct exists, then call get credit limit """
    def set_credit_limit(self, account_number, acctype, credit_score):
        with conn:
            account_found = BankAccount.get_account(self, account_number, acctype)
            if account_found:
                credit_limit = CreditCard.set_credit_limit_helper(self, account_number, credit_score)
                if credit_limit:
                    c.execute("""UPDATE credit_card SET credit_limit=:credit_limit
                            WHERE account_number =:account_number """,
                              {'account_number':account_number, 'credit_limit':credit_limit})
                    print("Account number {} credit limit is set to {}".format(account_number, credit_limit))    
            conn.commit()    


def get_credit_limit(self, account_number):
    with conn:
        c.execute("""SELECT credit_limit from credit_card WHERE account_number=:account_number""",
                  {'account_number':account_number})
        credit_limit = c.fetchone()
        if credit_limit is None:
            pass
        else:
            return(credit_limit[0])

""" get credit limit based on credit score passed in """
def set_credit_limit_helper(self, account_number, credit_score):

    if credit_score > 700:
        credit_limit = -2000

    elif credit_score > 100 and credit_score <= 300:
        credit_limit = -1500

    else:
        credit_limit = -1000
    return credit_limit

if __name__ == '__main__':

    obj1 =  CreditCard("Juan Santos", 135063555, 5544, 100, 'credit_card', 2200330066007700)
#    obj1.create_account("Juan Santos", 135063555, 9922, 100, 'credit_card', 2200330066007700)
    obj1.set_credit_limit(5544, 'credit_card', 200)
#    obj1.set_user_code(9922, 'credit_card', '[email protected]')
#    obj1.set_pin(9922, 'credit_card', 'b4493a59')

ALL of credit_card:

from bank_account import BankAccount
import sqlite3

conn = sqlite3.connect('bank_account.db')
c = conn.cursor()     

class CreditCard(BankAccount):
    def __init__(self, name, social, account_number, balance, acctype, card_no, credit_score=None, credit_limit=None):
        super().__init__(name, social, account_number, balance, acctype)
        self.card_no = card_no
        self.credit_score = credit_score
        self.credit_limit = credit_limit

    """ set credit limit, check if acct exists, then call get credit limit """
    def set_credit_limit(self, account_number, acctype, credit_score):
        with conn:
            account_found = BankAccount.get_account(self, account_number, acctype)
            if account_found:
                credit_limit = CreditCard.set_credit_limit_helper(self, account_number, credit_score)
                if credit_limit:
                    c.execute("""UPDATE credit_card SET credit_limit=:credit_limit
                            WHERE account_number =:account_number """,
                              {'account_number':account_number, 'credit_limit':credit_limit})
                    print("Account number {} credit limit is set to {}".format(account_number, credit_limit))    
            conn.commit()    

    def withdraw(self, account_number, acctype, amount):
        with conn:
            account_found = BankAccount.get_account(self, account_number, acctype)
            if account_found:
                balance = account_found[3]
                credit_limit  = CreditCard.get_credit_limit(self, account_number)
                amount_left = credit_limit - (int(balance))

                if balance - (int(amount)) < credit_limit:
                     print("Your balance is: {}, and your credit limit is: {}".format(balance, credit_limit))
                     print("The max you can withdraw is {}".format(amount_left))
                else:    
                    existing_bal = account_found[3]
                    c.execute("""UPDATE credit_card SET balance=balance -:amount
                            WHERE account_number =:account_number""",
                              {'account_number':account_number, 'amount':amount})
                    print("${} has been withdrawn from account {} and the new balance is ${}".format(amount, account_number, existing_bal - (int(amount))))

                    notif_set = BankAccount.get_notif(self, account_number, acctype)
                    if notif_set is None:
                        print("No notifications have been set for this acct")
                    else:
                        notif_withdraw = notif_set[7]
                        if notif_withdraw == 1:
                            notify = BankAccount.send_email(self,account_number, acctype, 'Withdraw', amount, existing_bal - amount)
            conn.commit()

    def get_credit_limit(self, account_number):
        with conn:
            c.execute("""SELECT credit_limit from credit_card WHERE account_number=:account_number""",
                      {'account_number':account_number})
            credit_limit = c.fetchone()
            if credit_limit is None:
                pass
            else:
                return(credit_limit[0])

    """ get credit limit based on credit score passed in """
    def set_credit_limit_helper(self, account_number, credit_score):

        if credit_score > 700:
            credit_limit = -2000

        elif credit_score > 100 and credit_score <= 300:
            credit_limit = -1500

        else:
            credit_limit = -1000
        return credit_limit

Adding send email program also:

import smtplib, sqlite3
from email.mime.text import MIMEText
from email.mime.multipart import MIMEMultipart
from email.mime.base import MIMEBase
from email import encoders
import bank_account

conn = sqlite3.connect('bank_account.db')
c = conn.cursor()

""" get email config - check if there is an existing acct first """
def get_email_config(account_number, acctype, action, amount, new_bal, name):

    subject = '{} account {} notification'.format(acctype, action)
    body = ''
    acctype = acctype.replace('_',' ')

    if action == 'Balance':
        body = '{},\n \
        The balance for account number: {} is : ${} '.format(name, account_number, amount[0])

    elif action == 'Deposit':
        body= '{},\n \
        A deposit of: ${} has been made to account number: {}\n \
        The new balance is now: ${}'.format(name, amount, account_number, new_bal)

    elif action == 'Overdraft':
        body = '{},\n \
        Please note that account number: {} is now overdrawn \n \
        The balance is ${}. Please add funds to avoid penalties'.format(name, account_number, new_bal)

    elif action == 'Withdraw':
        body = '{},\n \
        Please note that a withdrawl in the amount of: ${} has been made from account number: {} \n \
        The balance is ${}'.format(name, amount, account_number, new_bal)
    return(subject, body)

def send_email( account_number, acctype, action, amount, new_bal, name):

    """ Pull subject and body from email_config """
    email_config = get_email_config(account_number, acctype, action, amount, new_bal, name)

    body = email_config[1]
    subject = email_config[0]
    email_user = '[email protected]'
    email_send = '[email protected]'
    email_pass = 'pass'
    msg = MIMEMultipart()
    msg['From'] = email_config[0]
    msg['To'] = email_config[1]
    msg['Subject'] = email_config[-2]

    msg.attach(MIMEText(body, 'plain'))
    server = smtplib.SMTP('smtp.gmail.com: 587')
    server.starttls()
    server.login(email_user, email_pass)
    text = msg.as_string()
    server.sendmail(email_user, email_send, text)
    server.quit


    if action == 'Deposit':                            
        print("Deposit Notification sent")

    elif action == 'Balance':
        print("Balance Notification sent")

    elif action == 'Overdraft':
        print("Overdraft Notification sent")

    elif action == 'Withdraw':
        print("Withdraw Notification sent")     

    else:
            pass
\$\endgroup\$
5
  • 1
    \$\begingroup\$ This code is not complete. For instance, the static class method ATM.get_user_code() is not defined anywhere. You are using an sqlite3 database, but no where have you shown the schema. Both of these things makes it impossible for us to run the code, so we are left wondering if we have a complete picture of the program structure. Lacking that, we really can't provide a good code review with any confidence. \$\endgroup\$
    – AJNeufeld
    Commented Mar 7, 2019 at 5:53
  • \$\begingroup\$ Thought I might run out space which is why i didnt add all of the data in all the programs \$\endgroup\$
    – anfield
    Commented Mar 7, 2019 at 14:45
  • \$\begingroup\$ The limit on code review was raised to 65535 characters. \$\endgroup\$
    – AJNeufeld
    Commented Mar 7, 2019 at 15:04
  • \$\begingroup\$ Thanks. I added much more so people have a better idea of what it contains. Also I added the sql which created the DB, however I didnt keep it fully up to date when adding columns so there might be a slight difference here and there \$\endgroup\$
    – anfield
    Commented Mar 7, 2019 at 15:06
  • \$\begingroup\$ @AJNeufield - added more detail. get_user_code does exist but I thought I might be over the limit so I didnt add \$\endgroup\$
    – anfield
    Commented Mar 7, 2019 at 15:12

2 Answers 2

4
\$\begingroup\$

Some suggestions:

  • Running this code through a linter such as flake8 will give you some hints towards producing more idiomatic code. You can also use Black to automatically format your code to be more idiomatic.
  • You don't need to assign obj1 in main - the variable is unused.
  • main_menu taking a bunch of strings and numbers makes the call completely unintelligible on its own. If you were to instead create a Customer with an Account, Identification and BankCard instead, the meaning of the now single parameter would be obvious.
  • fetchone does not assert that there is only one record. You should make sure to check that whenever you use it outside of a loop you assert that there are no more rows after retrieving it. Otherwise you can very easily get into situations where business rules such as having only one code per card is broken.
  • Optional parameters are a code smell. That doesn't mean they are always bad, but they are very often a sign that the code needs some YAGNI love. In your case you never call main_menu without card_no, so it should not be optional.
\$\endgroup\$
4
  • \$\begingroup\$ Thanks. Would Customer, Account, Identification and BankCard be classes? \$\endgroup\$
    – anfield
    Commented Mar 7, 2019 at 15:45
  • \$\begingroup\$ Yes, hence the capitalization. \$\endgroup\$
    – l0b0
    Commented Mar 7, 2019 at 19:18
  • \$\begingroup\$ In that case is the customer basically replacing bank account, the superclass? Would the others be subclasses? And then taking pieces out of bank account and credit card etc, and putting these into Account, Identification etc where appropriate \$\endgroup\$
    – anfield
    Commented Mar 8, 2019 at 2:56
  • \$\begingroup\$ Also, are you saying main_menu would take one parameter or all the above main_menu(Customer, Account, Identification, BankCard) \$\endgroup\$
    – anfield
    Commented Mar 8, 2019 at 3:00
2
\$\begingroup\$

Layers

Perhaps the biggest problem with this project is a lack of clear layers of responsibility. You’ve got files and classes for organization, but it is highly coupled — which is not a good thing.

I would suggest reorganizing the code into layers:

  1. Presentation Layer: the UI. Presenting information to the user, and asking the user for information
  2. Business Logic. Checking if the correct pin/authorization code has been provided, verifying withdrawals don’t cause balances to go negative, ensuring deposits and withdrawals aren’t negative, etc.
  3. Data Model. Your BankAccount, and CreditCard goes here. They just store information. But these are “dumb” classes; little or no logic here.
  4. Data Access Layer: This is your connection to the database. You read/write BankAccount and CreditCard information here.

Why?

Presently, 6 of your 7 files import sqlite3. If you need to change your database to “MySQL” or “DB2”, you will need to touch almost all of your code. That is difficult and error prone. If the sqlite3 was contained in just one file, and that was the only thing in that file, you could easily switch out the database by switching out that one file.

Almost every file prints something too. What if you need to change your UI from console input/output to some kind of GUI, say based on tkinter. Again, you have to change almost every file. If the UI was separate from the business logic, data model and data access layer, it would be easy to do. As it presently stands, the entire application would change.

Ideally, each layer would be in a different package, but that might be overkill. Simply, “these 3 files are the business logic, these 4 files are for the UI, these 3 are the data model, and this file is the data access layer” would go wonders for decoupling the application, and allowing an easier time adapting, upgrading it, or simply maintaining it.

Doc Strings

You’ve tried to add """doc-strings""" to your code. I applaud the effort. But unfortunately you are doing it wrong.

Docstrings are simply strings that appear as the first statement of a function, class, and/or module. While they can be triple-quoted or single-quoted strings, and they can use single or double quotes, what matters is they are the first statement.

In a Python shell, type:

>>> def myfunc(a, b, c):
...     """A cryptic function name for a function that returns
...     the geometric mean of 3 numbers."""
...     return (a * b * c) ** (1 / 3)
...

Then, ask for “help” on the function you just created:

>>> help(myfunc)

The help command looks for a doc-string attached to the function, if any, and displays that information. Try help(abs) or help(map) or help(str.endswith). Useful information? Any guesses how it got there?

You have 3 triple-quoted strings before set_notifications. The benefit of triple-quoted strings are they can contain new-lines. Moving the lines into the set_notifications function will result in just the first line becoming the doc-string. You need to start the string with a triple-quote and end it three lines later with a triple-quote in order for the entire 3 lines of text to become the doc-string.


What is the point of print("""""""ATM Menu, choose an option""""""")? The first """ is ended by the next """, creating an empty string, then you have "ATM Menu, choose an option", followed by """ and """ which again creates another empty string. These 3 string are concatenated together giving you simply "ATM Menu, choose an option".

\$\endgroup\$
4
  • \$\begingroup\$ thanks for taking the time to comment. I will definitely take this into account \$\endgroup\$
    – anfield
    Commented Mar 8, 2019 at 14:33
  • \$\begingroup\$ thanks for commenting. Is there any way you can share some more detail on how I can go about splitting my code into layers as you mentioned? I am having trouble visualizing how I am going to accomplish this. One thing I did start doing was to take get_pin set_pin etc out and put it into its own class Identification. \$\endgroup\$
    – anfield
    Commented Mar 13, 2019 at 20:03
  • \$\begingroup\$ You've got several files, with typically only one class/file. One class/file is not pythonic; you end up with classes like bank_account.BankAccount, which you then work-around using from bank_account import BankAccount. You could restructure into several files, with several classes/file. One file, called model.py might contain several dumb, plain-old-data class structures for your various account types. Another file database.py could import model and provide your SQL queries for loading/saving the model data to the database. Another file business.py adds business logic, etc. \$\endgroup\$
    – AJNeufeld
    Commented Mar 13, 2019 at 21:37
  • \$\begingroup\$ Ive put the classes into their own file (account.py) and then create_account, get_account etc in another file, but that second file, database.py wants the class definitions in it in order to run. I'm not sure how to pass those definitions between the two \$\endgroup\$
    – anfield
    Commented Mar 14, 2019 at 14:24

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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