Today I reviewed a one line PR (plus some tests) that got me thinking about API design and communication between two different systems. What code should live in which system? Ultimately (as is so often in software design), this comes down to deciding the best place for some logic to live, where change becomes easy to make.

The change under review was this:

   def overdue?
+     return false if status.nil?
+ 
      status.label != 'ok'
    end

Site Q has the concept of a ServiceHistoryRecord. A car has to get an MOT every once in a while, and if it doesn’t that impacts its resale value (by quite a lot! don’t miss an MOT!). The ServiceHistoryRecord has a status: a string like ‘ok’, or ‘overdue_mileage’, or ‘overdue_date’. Essentially, if it’s not okay, it’s overdue. That’s what the code above does.

The new change is that sometimes the ‘status’ isn’t actually set. If not, assume it’s not overdue. This is an edge case that just needed to be watched out for, and it has only ever failed once, requiring this change.

Here’s the thing though: this change above is in Site D. Site Q owns this data, and status is actually an object from the Site Q API client. Is this code in the right place? Should the overdue? method be on the client object instead?

Site D is just showing the service history data. It doesn’t really know anything about what a service history record is, only that it needs to display it. There’s a good argument that overdue? is a complexity of the business domain, and that lives in Site Q. Only Q really needs to be aware that the status is sometimes missing, and what that means to the system.

The strongest reason for this being in Q is ease of change though. If we add another value to status, like ‘two_weeks_until_overdue’, then suddenly our overdue? method in D will start failing. We need to remember to patch this method before the new status can be used. We also need to remember that this method exists and where it’s being used.

A status.overdue? method would improve a number of things: ownership of the business logic would be clearer, the meaning of overdue would be easier to discover, and we’d have one less piece of domain knowledge duplicated in consuming applications.

At this point on a Monday, with the team handling the original error as a firefighting effort, and The Big Launch Of Another Feature coming up, is now the time to tackle this tech debt? Another question for another time. Or maybe all this needs to be is a reminder about ownership principles, and next time will be better. Big questions.

So, anyway, I approved the change. It’ll probably be fine.