Python Object-Oriented Programming fundamentals and the most common mistake

Posted Apr 25, 2021

By Sebastian Buczyński 3 min read

The problem

One antipattern I see over and over again in OOP Python code (e.g. with service layers in Django) is mutating attributes from the outside of a given object:

# more like pseudocode 
def place_bid(
    auction_id: AuctionId, amount: Money, bidder_id: BidderId
) -> bool:
    auction = AuctionRepo.get(auction_id)

    if amount <= auction.current_price:
        raise BidTooLow
    new_bid = Bid(amount, bidder_id)
    auction.bids.append(new_bid)  # ouch!
    auction.bids.sort(lambda key: -amount)  # ouch x2
    auction.current_price = auction.bids[0].amount  # ouch x3

    AuctionRepo.save(auction)

    is_winner = auction.bids[0].bidder_id == bidder_id
    return is_winnger

What's wrong with that? Surprisingly many things. First of all, it's pretty complex logic that touches the guts of an Auction. It does not only mutate the state of an auction but also enforces certain invariants. The latter are conditions that always have to be met and they root in business rules. For example, the current price is equal to the amount of the top bid. Auction is much better for having this logic than some view or service function.

If it's not apparent, imagine we have to implement withdrawing a specific bid. Once again, the state of an Auction would be mutated and the current price has to be recalculated. Plus there is one edge case - if the last bid is withdrawn, current_price should be reset to auction's open price.

# pseudocode again
def withdraw_bid(amount: Money, bidder_id: BidderId) -> None:
    auction = AuctionRepo.get(auction_id)

    bid_to_withdraw = Bid(amount, bidder_id)
    auction.bids = [  # OMG
        bid for bid in auction.bids if bid != bid_to_withdraw
    ]
    try:
        auction.current_price = auction.bids[0].amount
    except IndexError:
        auction.current_price = auction.open_price

    AuctionRepo.save(auction)

Don't tell me you like this procedural code.

Meet Tell, don't Ask

Now, a quick reminder what Object-Oriented Programming is:

In short, we bring together data and behaviour during design - usually in a form of classes. However, using classes doesn't make a code OOP! Also, the opposite is true - OOP doesn't necessarily use classes. Especially in Python, where everything is an object and sometimes a module (an object!) with methods inside would also do.

There is Tell, Don't Ask principle that states

Tell-Don't-Ask is a principle that helps people remember that object-orientation is about bundling data with the functions that operate on that data. It reminds us that rather than asking an object for data and acting on that data, we should instead tell an object what to do. This encourages to move behavior into an object to go with the data.
https://martinfowler.com/bliki/TellDontAsk.html

The design encouraged by Tell, Don't Ask would indeed burded Auction with all that logic, so our views/services would be simpler:

# pseudocode x 3
def place_bid(
    auction_id: AuctionId, amount: Money, bidder_id: BidderId
) -> bool:
    auction = AuctionRepo.get(auction_id)

    auction.place_bid(amount, bidder_id)

    AuctionRepo.save(auction)

    return auction.is_winner(bidder_id)


def withdraw_bid(amount: Money, bidder_id: BidderId) -> None:
    auction = AuctionRepo.get(auction_id)

    auction.withdraw_bid(amount, bidder_id)

    AuctionRepo.save(auction)

All of the operations appending bids or doing whatever to fields of the auction will now be methods on Auction class. place_bid and withdraw_bid now only tells Auction what to do, it doesn't ask for its data to directly change it.

Notice that now code of place/withdraw_bid adheres to favoured Python's coding style -

Easier to ask for forgiveness than permission

You don't have to do extra checks from the outside - Auction wil tell you if operation cannot be performed, either by return value or by raising a domain-specific (e.g. NoSuchBidToWithdraw or BidTooLow) exception :)

Is it always bad to ask?

No, if you are writing a script and objects you're mutating are not domain models but rather data structures. In simple data processing where dictionaries or data classes fly around adding methods to them would be overkill. Often, there are no invariants we could or should protect. In such cases, we should be totally fine by not using OOP.

Summary

Object-Oriented Programming excels when we model complex domains with invariants to protect and many nuances. Tell, Don't Ask principle encourages Object-Oriented thinking and design. It helps eliminate dangerous code & logic duplication before it occurs. It also plays really nicely with the idea of encapsulation.

Don't make your objects dumb bags of data when they represent a domain concept :)