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
if currentTime = alarmTime then ringAlarm end if
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.
Line | Horizon code sample | |||||
---|---|---|---|---|---|---|
1 | If lstockrootnode = 3013 Or lstockrootnode = 3016 Then | |||||
2 |
bremedprods = False | |||||
3 |
intbalancerootlevel = 5 |
|||||
4 |
lbalancerootenode = 3017 |
|||||
5 |
If lstockrootnode = 2493 Then |
|||||
6 |
bremedprods = False |
|||||
7 |
intbalancerootlevel = 3 |
These pointless red lines | These green lines | |||
8 |
lbalancerootenode = 3006 |
of code are never used | of code are | |||
9 |
End If |
confusingly the same | ||||
10 |
Else |
|||||
11 |
bremedprods = True |
|||||
12 |
intbalancerootlevel = 5 |
|||||
13 |
lbalancerootenode = 3017 |
|||||
14 |
End If | |||||
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?
// This code does ??? (if only I knew what it did I’d say!) |
bremedprods = lstockrootnode ≠ 3013 And lstockrootnode ≠ 3016 |
intbalancerootlevel = 5 |
lbalancerootenode = 3017 |
… This does the same thing as the original Horizon code above — though I still don't know what it’s trying to achieve.
Whatever the original code did, because I’ve rewritten the code to be shorter and clearer (and it’s lost all the distracting unreachable and duplicate code) it’s now much easier to understand and spot any bugs (like maybe 3013 or 3017 are typos?).
For the same reasons, my version of the code is also much, much easier to maintain if and when bugs become apparent — which, given the many known failures in Horizon, is very important.
© Harold Thimbleby, 2024.
With thanks to P Marshall, J Christie, PB Ladkin, B Littlewood, S Mason, M Newby, J Rogers, & M Thomas.