How to draw programmer's attentions in certain conditions?

Question

Let's start with an example.

Let's say, I have a method called export that depends heavily on the DB schema. And by “depends heavily” I mean I know that adding a new column to a certain table often (very often) leads to the corresponding export method change (usually you should add the new field to the export data as well).

Programmers often forget to change export method since it's not really clear you should even look at this. My goal is to force programmer explicitly make a decision to determine whether he forgot to look at the export method or just don't want to add a field to the export data. And I'm looking for the design solution for this problem.

I have two ideas, but both of them have flaws.

Smart “Read all” wrapper

I can create the smart wrapper that makes sure all data is explicitly read.

Something like this:

def export():
    checker = AllReadChecker.new(table_row)

    name    = checker.get('name')
    surname = checker.get('surname')
              checker.ignore('age') # explicitly ignore the "age" field

    result = [name, surname] # or whatever

    checker.check_now() # check all is read

    return result

So, checker asserts if table_row contains another fields that were not read. But all this thing looks kind of heavy and (maybe) affects perfomance.

“Check that method” unittest

I can just create the unittest that remembers the last table schema and fails every time the table is changed. In that case programmer would see something like “don't forget to check out the export method”. To hide the warning programmer would (or wouldn't — that's a problem) check out export and manually (that's another problem) fix the test by adding new fields into it.

I have a few other ideas but they are too troublesome to implement or too difficult to understand (and I don't want the project to become a puzzle).

---

The above problem is just an example of the more wide class of problems I encounter from time to time. I want to bind some pieces of code and/or infrastructure so changing one of them immediately alerts programmer to check another one. Usually you have some simple tools like extracting common logic or writing reliable unittest, but I'm looking for the tool for more complex cases: maybe some design patterns I'm now aware of.

Answer 1

You're on a right track with your unit test idea, but your implementation is wrong.

If the export is related to the schema and the schema changed, there are two possible cases:

• Either the export still works perfectly well, because it was unaffected by a slight change in the schema,

• Or it breaks.

In both cases, it's the goal of the build to track down this possible regression. A bunch of tests—would it be integration tests, or system tests, or functional tests or something else—ensure that your export procedure works with the current schema, independently of the fact that it changed or not since the previous commit. If those tests pass, great. If they fail, this is a sign to the developer that he may have missed something, and a clear indication where to look.

Why is your implementation wrong? Well, for several reasons.

1. It has nothing to do with unit tests...

2. ... and, actually, it's not even a test.

3. The worst part is that fixing the “test” requires to, well, actually changing the “test”, that is doing an operation which is completely unrelated to the export.

Instead, by making actual tests for export procedure, you make sure that the developer will fix the export.

---

More generally, when you encounter a situation where a change in one class always or usually requires a change in a completely different class, this is a good sign that you did your design wrong and are violating the Single Responsibility Principle.

While I talk specifically about classes, it applies more or less to other entities as well. For instance, a change in database schema should be either reflected automatically in your code, for instance through code generators used by many ORMs, or should at least be easily localized: if I add Description column to Product table and I use no ORMs or code generators, I at least expect to make a single change within Data.Product class of the DAL, without the need to search through all the code base and find some occurrences of Product class in, say, presentation layer.

If you can't reasonably restrict the change to one location (either because you're in a case where it simply doesn't work, or because it requires a huge amount of development), then you create a risk of regressions. When I change class A, and class B somewhere in the code base stops working, it's a regression.

Testing lowers the risk of regressions and, which is far more important, shows you the location of a regression. This is why when you know that changes in one location cause issues in a completely different part of the code base, make sure you have enough tests which raise alarms as soon as a regression appears at this level.

In all cases, avoid relying in such cases just on the comments. Something like:

// If you change the following line, make sure you also change the corresponding
// `measure` value in `Scaffolding.Builder`.

never works. Not only developers won't read it in most cases, but it will often end either removed or moved far away from the concerned line, and become impossible to understand.

Answer 2

It sounds to me like your changes are underspecified. Say you live somewhere that doesn't have postal codes, so you don't have a postal code column in the address table. Then postal codes are introduced, or you start dealing with customers who live where there are postal codes, and you have to add this column to the table.

If the work item just says "add postal code column to address table" then yes, the export will be broken, or at least won't export postal codes. But what about the input screen that is used to enter postal codes? The report that lists all the customers and their addresses? There are tons of things that need to change when you add this column. The job of remembering those things is an important one - you shouldn't be counting on random code artifacts to "trap" developers into remembering.

When the decision is made to add a meaningful column (that is, not just some cached total or denormalized lookup or other value that doesn't belong in an export or report or on an input screen) the work items created should include ALL the changes needed - adding the column, updating the populate script, updating the tests, updating the export, the reports, the input screens and so on. These may not all be assigned to (or picked up by) the same person, but they must all get done.

Sometimes developers choose to add columns themselves as part of implementing some larger change. For example someone may have written a work item to add something to an input screen and a report, without thinking about how it is implemented. If this happens a lot, you will need to decide whether your work-item-adder needs to know implementation details (so as to be able to add all the right work items) or whether developers need to be aware that the work-item-adder sometimes leaves things out. If it's the latter then you need a culture of "don't just change the schema; stop and think about what else that affects."

If there were a lot of developers and this happened more than once, I would set up a checkin alert for the team lead or other senior person to be alerted on schema changes. That person could then look for related work items to deal with the consequences of them schema change, and if the work items were missing, could not only add them but educate whoever left them out of the plan.

Answer 3

Almost always, when creating an export I also create a corresponding import. As I have other tests which fully populate the data structure being exported, I can then build a round-trip unit test which compares a fully populated original with an exported then imported copy. If they are the same, then the export/import is complete; if they are not the same then the unit test fails and I know that the export mechanism needs updating.