#2 - Going to second base: know your boundaries

... or you'll end up with a monkey patching your code.

So, we've got to get intimate...

For today's cautionary tale, let me draw a parallel. You've gone on a few dates, all is going well, but you're not sure if you should go home with your date. After all, you hardly know them. How do you navigate that?

Over the last couple of articles, we've gotten acquainted with the code that the LLM wrote, but we haven't read a single line yet. If there were huge red flags, maybe we've spotted some already, but could there be something more sinister hiding in plain sight? Is it time to open up the files and look at actual code?

Well, hopefully you've sorted out the structural issues with your project, so let's go and ask a few probing questions...

What was your name again?

Ouch. If I had a dollar for every time I forgot the name of someone that I should know... But naming things turns out to be quite important for us too. Let's grab a poorly written Python method signature as an example:

def process_order(order_data)

What was his name again... Jim, Jeff, Geoff, Jean? Well... you get what I mean, hopefully, and I wouldn't fault you for not knowing what this method does, either.

@dataclass
class Order:
    customer: Customer
    items: list[OrderItem]
    discount_code: DiscountCode | None = None

@dataclass 
class ProcessedOrder:
    order: Order
    total: Decimal
    shipping: Decimal
    processed_at: datetime
    discount_applied: DiscountCode | None
    inventory_reserved: bool

class OrderProcessor:
    def __init__(
        self,
        discount_rules: DiscountRules,
        shipping_policy: ShippingPolicy,
        inventory: InventoryService,
    ):
        ...

    def process(self, order: Order) -> ProcessedOrder:
        ...

Without even writing any of the implementation, I've already given you enough data to have a plausible opinion about what it should do. You could then likely test it without really looking at what's inside, and that's what matters. If you know the business logic, and have detailed signatures like we have above, then you're ready to move on to the next step!

Size matters

Well, we've got a process method with some boundaries now, but we're still not looking at instructions. Let's assume process is a large method. Inevitably... it might well be if it's trying to do too much at the same time.

So we've got to put some limits too, and maybe we don't want the process method to worry about too many things at once. Maybe just calling different methods that worry about individual elements like discounts, shipping, and so on.

    def _calculate_total(self, items: list[OrderItem]) -> Decimal:
        ...

    def _apply_discount(self, total: Decimal, code: DiscountCode) -> Decimal:
        ...

    def _determine_shipping(self, total: Decimal) -> Decimal:
        ...

    def _reserve_inventory(self, items: list[OrderItem]) -> bool:
        ...

Phew. Now everything's settled. That's a much better structure to see in your generated code. If it feels like a method is a bit long, or doing more than one clear thing, it probably needs looking at. Even without reading any of the code, I can tell you that I'd expect something looking roughly like the above for such an order processing class.

It pays to have taste

Also, did I mention how nested loops and continue / break keywords are a pet peeve of mine? Yeah, that's a red flag too. You don't want to be nesting too many things at once. In code, or dating. Believe me.

I did mention above that I'd expect a few methods to be present to break the flow, but in the end it's experience talking. You have to read and write code a fair bit to get there, but that's part of the beauty of this job. However, when you do refactor or write code, there's a few constraints you can put on yourself, which may be helpful in getting to a better state and therefore learn as you do.

def get_eligible_totals(self, orders: list[Order]) -> list[Decimal]:
    totals = []
    for order in orders:
        if order.discount_code is None:
            continue
        if not self._discount_rules.is_valid(order.discount_code):
            continue
        total = Decimal("0")
        for item in order.items:
            if item.qty <= 0:
                continue
            if item.price <= 0:
                break
            total += item.price * item.qty
        if total > 0:
            totals.append(total)
    return totals

What's going on there? Well, you tell me. But if that hurts your brain, don't spend too much time on the above and read what follows:

def get_eligible_totals(self, orders: list[Order]) -> list[Decimal]:
    eligible = [o for o in orders if self._has_valid_discount(o)]
    totals = [self._calculate_total(o.items) for o in eligible]
    return [t for t in totals if t > 0]

def _has_valid_discount(self, order: Order) -> bool:
    ...

def _calculate_total(self, items: list[OrderItem]) -> Decimal:
    ...

Did you notice the bug in the first method, too? In the original, break exits the inner item loop entirely. So if the third item has a negative price, items four, five, and six never get counted. Knowing what your language allows (in this case list comprehensions) and avoiding smelly keywords (global, break, continue, etc...) will get you there, at least some of the way.

datetime.now(), I hardly knew thee

So now that we've got a feel for structure and taste, let's talk about the things hiding deeper. The places where your code reaches out and touches the real world.

Time is the classic offender. A datetime.now() buried three layers deep means your code behaves differently every time you run it. Randomness is the same problem wearing a different hat. Configuration is a bit more predictable but can still sneak up on you. Then your tests start to break because it's January and someone decided to compare the months in a date object. I wish I was making this up.

IO, filesystem, network calls, external services, mutable state, user input, build environment... they're all boundaries too. Some obvious, some less so. But they all reduce to one question: can I control this from a test? If you can't swap it out, it's not a boundary, it's a leak. And would you trust a monkey with a wrench to fix it? Well, I wouldn't, and I wouldn't monkey patch it either.

Oh, and keep your eyes open for those red flags: nested loops, giant methods, untyped data, hardcoded values, hidden dependencies. They're the crumbs which feed the little bugs...

Robots have no taste... or if they did they'd be brutalists.

Someone else did it wrong at least once in the training data, and therefore, you must now suffer the consequences.

And hey, if you can explain those key common problems to your LLM and configure a competent software engineer AI agent as a result, you'll probably save yourself some time. But agents are a topic in and of their own which is better addressed in its own post.

Oh, did you notice how I didn't address the missing Clock in the OrderProcessor class? Well, that's something you'd want to inject in C# or other languages, but sometimes, no matter how much you think a particular Dutch person is wrong, there's no need to get angry and the best course of action is just let them drive slowly on the motorway. They're on holidays, and you can't worry about every time someone else is wrong.

Same goes for (some) Python conventions. So relax, chill out, and go along with freezegun.

Until next time...

~ Ben