Bank account balance, withdraw, deposit: compartmentalize without losing functionality

Question

Is there a better way to write this while maintaining its current functionality? (checking balances, updating, subtracting, etc...) If so, where would I begin? I saw a code briefly in class that was for a similar questions and it looked more streamlined.

class Account:

    # constructor
    def __init__(self, name, account_number, balance):
        self.name = name
        self.account_number = account_number
        self.balance = balance

    # returns string representation of object
    def __str__(self):
        return"Account Name: {0}\nAccount Number: {1} \nAccount Balance:\
    ${2:.2f}".format(self.name, self.account_number, self.balance)

    # add given amount to balance
    def deposit(self, amount):
        self.balance += amount

    # subtract amount and fee from balance
    def withdraw(self, amount, fee):
        self.balance = self.balance - amount - fee

if __name__ == '__main__':
    # make 3 objects
    acct1 = Account('Guy Mann', 90453889, 100)
    acct2 = Account('Donald Duck', 83504837, 100)
    acct3 = Account('Joe Smith', 74773321, 100)

    # print
    print(acct1)
    print(acct2)
    print(acct3)

    # deposit and print
    acct1.deposit(25.85)
    acct2.deposit(75.50)
    acct3.deposit(50)
    print(acct1)
    print(acct2)
    print(acct3)

    # withdraw and print
    acct1.withdraw(25.85, 2.50)
    acct2.withdraw(75.50, 1.50)
    acct3.withdraw(50, 2)
    print(acct1)
    print(acct2)
    print(acct3)

Answer 1

This code is already quite compact. Some PEP8 code styling will not hurt. Some spaces here and there, consistency in using quotes in string literals. Also some comments can be turned into docstrings. (and comment like "constructor" is not necessary - everyone who knows Python will recognize that).

However, one thing probably good to make sure: The balance better use proper type, for example, decimal. Float is terribly wrong type for monetary transactions.

Answer 2

I agree with Roman's answer in that the code is already compact. In it's current configuration though, it is not very scalable. There is a lot of duplicate code in __main__, and you could replace the Account creation statements with a dictionary of tuples and a for-loop. Then you could store the Account instances back into a dictionary. Using a dictionary would allow you to reference the accounts by the name of the holder rather than by free-standing variables, which seems semantically cleaner.

Additionally, the print statements could be replaced with a for-loop as well.

These modifications may not reduce the number of lines in the code as is, but if you consider a case where you wanted to make 100 Accounts and print information about them, then the approach above may be better.

Answer 3

The __str__ method can be simplified a bit. Already with a normal format string you do not need to specify the index of the object to format if it is just sequentially:

"Account Name: {}\nAccount Number: {}\nAccount Balance: ${:.2f}".format(self.name, self.account_number, self.balance)

You could also use dictionary lookup:

"Account Name: {self.name}\nAccount Number: {self.account_number} \nAccount Balance: ${self.balance:.2f}".format(self=self)

And then you could use the relatively new f-string (Python 3.6+), making it a bit shorter:

def __str__(self):
    return f"Account Name: {self.name}\nAccount Number: {self.account_number}\nAccount Balance: ${self.balance:.2f}"

And then, finally, use a multiline string to make it fit into 80 character lines:

def __str__(self):
    return f"""Account Name: {self.name}
Account Number: {self.account_number}
Account Balance: ${self.balance:.2f}"""