An example of problematic Post Office Horizon code

A very basic part of programming is the conditional statement, which works much like the conditional in English. The programmer says something like “if something is true then do this or else do this.”

Your alarm clock (at least if it has a computer in it) will have some code something like

The following lines of Post Office Horizon code were disclosed in a report to the Post Office Inquiry. I don’t know what this code does, because the code doesn’t say what it is doing (which is very bad practice), and the code doesn’t say what any of the numbers mean (also very bad practice). But regardless of what it’s supposed to do, we can see this is terrible code.

I’ve added colours and line numbers to help explain some of the problems.

Whatever the code is trying to do, we can see that there are two conditionals here. Line 1 says “if”, line 10 says “else” and line 14 says “end if” and that makes up the main conditional, however inside this main conditional is another one, running over lines 5 to 9, which I’ve coloured in red.

Line 5 says if something called lstockrootnode is equal to 2493 then the code in lines 6, 7 & 8 will happen. That might sound reasonable, but since line 1 establishes that lstockrootnode cannot be equal to 2493 here (over lines 2 to 9), and that means the code I’ve highlighted in red simply cannot happen. The technical term is that this is dead code. Dead code is pointless, but it isn’t just pointless, it’s confusing — a programmer may waste time understanding it, when it is not even used by Horizon. Presumably a programmer wrote this code because they thought it was needed ; if so, they have made a mistake as the needed code can never be run by the computer. It’s an elementary mistake.

Now notice that the two sets of lines that I coloured green (lines 3 & 4, and 12 & 13) are exactly the same. Yet they are in different arms of a conditional, so they happen regardless. In other words, these lines should have been outside the conditional, so nobody thinks what happens depends on the conditional test. It means (if they are supposed to be the same!) that the programmer has 4 lines of code that must checked are in fact the same, and they must remember to check, as the code doesn’t say so. It would have been far better to write the 2 lines of code just once, before or after the conditionals, whichever is clearer. It’s another elementary mistake.

Professional programmers will be surprised by these flaws in the programming. If these few lines are so worrying, just how bad is the rest of Horizon?