Basic Python OO bank account

Question

I am learning Python and in particular learning about classes. Since I'm self teaching, I'd appreciate any feedback and constructive criticism.

I have a couple specific questions:

1. In Python you can have multiple functions and classes in a single file (module), but is it considered bad practice to do so?
2. Python doesn't have private, so is there a point to having accessor methods like I do below?
3. My IDE suggest I put a comment beneath each function signature. Is it still good to do this when the functions are short and simple, like withdraw(x) in a bank account?

BankAccount.py

# bank account class
class BankAccount:
    def __init__(self, name, balance=0.00):
        self.name = name
        self.balance = balance

    def deposit(self, amount):
        """make a deposit"""
        self.balance += amount

    def withdraw(self, amount):
        """make a withdraw"""
        if amount > self.balance:
            raise ValueError("insufficient funds")
        self.balance -= amount

    def get_balance(self): #are accessor methods needed in Python?
        """check the balance"""
        return self.balance


def main():
    customer1 = BankAccount('Alex')
    print(customer1.get_balance())
    customer1.deposit(100)
    customer1.withdraw(30)
    print(customer1.get_balance())

    customer2 = BankAccount('Sam', 200)
    print(customer2.get_balance())

if __name__ == "__main__":
    main()

Answer 1

Python doesn't have private anything but we still have a need to separate inners of a class from the public interface.

By convention, attributes or methods whose name starts with a single underscore are considered internals of the class and, thus, not part of the public API. There is no mechanism in place to restrict their access, so user that know what they are doing can still use them; but if anything goes wrong, it will most likely be their fault, not yours.

Also, in the context of versionned libraries, internals are not subject to backward compatibility and can change without notice on a new release.

But instead of methods as getter or setters, we prefer properties that allows the same kind of flexibility (read-only vs. read-write) with the simplicity of attributes access.

Appart from that, I would add either __str__ or __repr__ (or both) as methods in your class to simplify printing accounts:

class BankAccount:
    def __init__(self, name, balance=0.00):
        self.name = name
        self._balance = balance

    def deposit(self, amount):
        """make a deposit"""
        self._balance += amount

    def withdraw(self, amount):
        """make a withdraw"""
        if amount > self._balance:
            raise ValueError("insufficient funds")
        self._balance -= amount

    @property
    def balance(self):
        """check the balance"""
        return self._balance

    def __repr__(self):
        return '{0.__class__.__name__}(name={0.name}, balance={0.balance})'.format(self)

    def __str__(self):
        return 'Bank account of {}, current balance: {}'.format(self.name, self.balance)

Example usage being:

customer1 = BankAccount('Alex')
print(repr(customer1))
customer1.deposit(100)
customer1.withdraw(30)
print(customer1)
customer2 = BankAccount('Sam', 200)
print(customer2.balance)

---

As regard to the "one class per file" rule, it may make sense in languages where a file does not bear much meaning.

But in python a file is a module. It is perfectly fine and expected to put related entities in them as they define a namespace.

Beside, where would you put simple functions and constants using a "one class per file" approach? As class attribute of some sort of "namespace" class? Yuck! Module already serve this purpose, don't reinvent the wheel.

Answer 2

In this answer I'm not going to review your code, but instead discuss the design of a book-keeping system. (I know that you're just using a bank account as an exercise, but an important part of programming is thinking about design like this.)

---

The code in the post implements account objects with methods for transactions. For example here's a transaction affecting the account customer1:

customer1.deposit(100)

But where did this money come from? Real book-keeping systems are double entry — that is, every transaction affects two accounts. If the money came from the account customer2 then there must be a corresponding withdrawal on that account so that the accounts balance.

But now we have to worry about atomicity of transactions. In order to process the transaction, we have to make two calls:

customer2.withdraw(100)
customer1.deposit(100)

but what if there's a power cut after the withdrawal and before the deposit? This would cause the money to be lost in transit.

For these reasons, the usual way to represent financial data is in the form of a ledger, which is a table of transactions:

FROM       TO         AMOUNT
---------  ---------  ------
customer2  customer1     100
customer1  cash           30

(Typically there would be other fields like the date and the authorizing user, to provide an audit trail.)

If the ledger supports atomic append (that is, trying to append a transaction to the ledger either succeeds or fails as a whole) then money can't be lost.

In a ledger system you would likely still have objects representing accounts, but an account balance would be a cache of the balance you get by applying the transactions in ledger.

Answer 3

1. In Python you can have multiple functions and classes in a single file (module) but is it considered bad practice to do so?

You can write as many functions and classes within a Python module as long as they serve the same purpose. For instance, you can have in a module these functions:

• increase_speed()
• decrease_speed()
• brake()
• stop_vehicle()
• start_vehicle()
• turn_left()
• turn_right()

What is considered a bad practice is to add a function like eat_mushrooms() which obviously needs to be in a separate module because of reasons you may have guessed already.

2. In Python they don't have private. So is there a point to having accessor methods like I do bellow?

As highlighted in the previous answer, the conversion is that a private data's name must start with an underscore _ and use @property annotation instead of setters and getters you have learnt elsewhere.

3. My IDE suggest I put a comment beneath each function signature. Is it still good to do this when the functions are short and simple, like withdraw(x) in a bank account?

A well written code does not need lot of comments (or function documentation). That is also the case with small functions (and functions have to be small according to C. Martin in Clean Code)

About your code

1. a) That comment (# bank account class) is not adding any value to your code because the class name highlights that idea already. So do not repeat yourself. That comment is a distraction for the reader of your code.

2. b) Usually you should treat __init__() as other functions: leave one blank line between the class name and this initializer.

3. """make a withdraw""" useless and should be removed for the same reason as in 1.a)

4. """make a deposit""" useless and should be removed. Please read 2.

5. For the __init__() use as less parameters as you can (C. Martin in Clean code) and from @GarethRees's answer, you should create an other function to set the customer's account like this:

   @property
   def customer_name(self):
       return self._customer_name
   
   @customer_name.setter
   def customer_name(self, customer_name):
       self._customer_name = customer_name
   

6. Use customer_name instead of name because name is quite confusing (name of a car? country?)

7. In __init__() you have a named parameter with a default value (balance = 0.00), because of the reasons mentioned in 5 you can get rid of that parameter. So your initializer becomes:

   def __init__(self):
      self._balance = 0.00
   

8. When your run customer1 = BankAccount("Alex") it is clear that BankAccount() is not a good class name for this context. So Customer() could be a better name. That way, customer1 = Customer("Alex") makes a sense.
9. Consequently to 7., you should rename the remaining functions as follows: deposit_in_bank_account(amount), withdraw_from_bank_account(amount).

From the previous remarks, your class becomes:

class Customer:

    def __init__(self):
        self._balance = 0.00
        self._customer_name = None

    def deposit_in_bank_account(self, amount):
        self._balance += amount

    def withdraw_from_bank_account(self, amount):
        if amount > self._balance:
            raise ValueError("insufficient funds")
        self._balance -= amount

    @property
    def balance(self):
        return self._balance

    @property
    def customer_name(self):
        return self._customer_name

    @customer_name.setter
    def customer_name(self, customer_name):
        self._customer_name = customer_name