Problem
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)
Solution
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.
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 Account
s and print information about them, then the approach above may be better.
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}"""