Good thing this dev quit. I'd have fired him. Out of a cannon. Into the sun

Line Break Roll up, roll up. It's your Wednesday dose of ridiculous code spotted in the wild.

If you've seen some horrors, send over your tales, please, and we'll share them with our readers. We've had a great response so far, which perhaps is a damning indictment for the software engineering industry. It can't be all bad, though – perhaps some snippets of amazing code are needed to even things out?

Oh OK, then, on with the misery.

You know NUMERICALNOTHING, Jon Snow

Here's a fantastic flame from a reader I'm going to refer to as 'A' in case the following gets him or her into trouble.

I've been working the last six months on a correcting some truly terrible sins in a VB.net project. The developer who wrote it is long gone (probably a good thing as I would want to see what could be done to fire him. Out of a cannon. Into the sun). He had the reputation of being somewhat of a talented programmer.

So to investigate those talents: firstly he'd switch both Option Explicit and Option Strict to OFF, disabling not only warnings, but also enforcement of data types. Secondly, he seemed to have no idea of the existence of nullable value types. Which in turn led to the third talent: each class began with the two magical lines:

Const NUMERICALNOTHING = -1 ' All Integers/Booleans are initialised as -1
Const BOOLEANNOTHING = 2    ' All Booleans are initialised as 2

Sigh. The mind boggles just quite what was being thought of here. I wish I could have a conversation with that developer and give him a piece of my mind. He clearly didn't have any himself.

Just think of the bugs possible with code unsure if -1 actually means -1 or means no value assigned. Disaster waiting to happen. Lesson: Don't pluck values out of the air and give them special powers – or your days, like your magic constants, could be numbered.

Left to my own devises

Next up, Andy writes in with:

I once worked on a mobile phone messaging client for a large Asian handset manufacturer, where the developers had regularly created defines to correct spelling mistakes in function names. This allowed them to type the name of the function how they thought it was spelled, and the code would still work, but resulted in us having half a dozen different names for common functions.

e.g. file1.h:

#define DeviseTextMeasure DeviceTextMeasure

file2.h:

#define DeviceTextMesure DeviseTextMeasure

etc.

It's the "etc" at the endd that nales it for thiss humble hacck, reveeling theres plentay more fixups to lewk out for. This supposad solushun to sperling funcshuns manes wrong jumps from a krazy one-off decishun to acktual p0licy. Lesson: I wouldn't even wish this kind of crap on my worst enemy.

Do as I say ... or else

Reg reader Dave confesses he ran headlong into this problem with nested if() statements – a classic blunder we've possibly all stumbled over from one time or another:

I stubbornly had to fight against the code formatting in Visual Studio and thought the debugger had gone mad when debugging:

if (isConfigured())
    if (isAllowed())
        doTask();
else
    errorNoConfiguration();

So if the code path is configured but is not allowed to perform the task, you'll get told there's a missing configuration? That'll blow someone's mind. Dave's on hand to deliver the lesson for this cockup:

So many morals:

• Visual flow does not always match true control flow
• Always use blocks ({})
• If the IDE formatter is formatting your code strangely your code might be the problem.

Thumbs up for {} blocks.

Array-bian night-mare

Finally, Alan leaves us with this cerebral tale of someone storing, midway through an algorithm, results of calculations smack bang alongside the input data. Not the end of the world, you might think, but the program expected the input data to be a fixed size, and so when the input array increased, things got messy. Take it away, Alan:

By far and away the most vile code I've seen has been from extremely smart people. They can skip the rules that we mere mortals obey out of common sense for fear of the consequences (like descriptive variable names) because the code is still understandable to them.

The worst case of this was when a brilliant, venerable and esteemed colleague passed away leaving behind about 1,100 lines of FORTRAN to calculate a particular statistic. He used no subroutines, despite the fact that the last thing he added was a Monte-Carlo approximation to the null hypothesis sampling error that required the reuse of the statistical calculations; he had used GOTOs to perform the Monte-Carlo calculations.

He also assumed that no more than 100 stimuli would ever be used, so he defined a matrix to hold the data indexed to 103 and he used the last three rows to store different intermediate terms. So you'd have lines like this:

XB(4,101)=XB(4,101)+XXP(101)*XXP(102)

where cells 1 to 100 held data, but 101 and 102 held particular intermediary terms (possibly different intermediary terms at different points in the code). Naturally, I needed to analyze more than 100 stimuli almost immediately.

The code I inherited was the seventh "version" and parts of it near the top of the file were well commented but there were virtually no comments in the second half of the file. Not a single GOTO or CONTINUE statement was commented.

Finally, this wasn't his fault, but he used an ancient Microsoft FORTRAN compiler that (a) had since been discontinued and (b) used proprietary language extensions. So, it was a real chore just to get his code to compile. I ended up rewriting it from scratch.

They say starting again from scratch is a terrible idea, but in Alan's case, we can make an exception. Lesson: Don't use naked magic numbers, least of all for special rows in input data that actually hold variable intermediate values. Like warring families at a wedding, just keep 'em separate.

Of course, we hope you'll join us next week for more Line Break. ®

Click here to see all Line Break columns