Jon recently started a new project. When setting up his dev environment, one of his peers told him, "You can disable verbose logging by setting DEBUG_LOG=false in your config file."

Well, when Jon did that, the verbose logging remained on. When he asked his peers, they were all surprised to see that the flag wasn't turning off debug logging. "Hunh, that used to work. Someone must have changed something…" Everyone had enough new development to do that tracking down a low priority bug fell to Jon. It didn't take long.

const DEBUG_LOG = process.env.DEBUG_LOG || true

According to the blame, the code had been like this for a year, the commit crammed with half a dozen features, was made by a developer who was no longer with the company, and the message was simply "Debugging". Presumably, this was intended to be a temporary change that accidentally got committed and no one noticed or cared.

Jon fixed it, and moved on. There was likely going to be plenty more to find.

Matt needed to add a new field to a form. This simple task was made complicated by the method used to save changes back to the database. Let's see if you can spot what the challenge was:

public int saveQualif(String docClass, String transcomId, String cptyCod, String tradeId, String originalDealId, String codeEvent, String multiDeal,
            String foNumber, String codeInstrfamily, String terminationDate, String premiumAmount, String premiumCurrency, String notionalAmount,
            String codeCurrency, String notionalAmount2, String codeCurrency2, String fixedRate, String payout, String maType, String maDate,
            String isdaZoneCode, String tradeDate, String externalReference, String entityCode, String investigationFileReference,
            String investigationFileStartDate, String productType, String effectiveDate, String expiryDate, String paymentDate, String settInstrucTyp,
            String opDirection, String pdfPassword, String extlSysCod, String extlDeaId, String agrDt) throws TechnicalException, DfException

That's 36 parameters right there. This function, internally, creates a data access object which takes just as many parameters in its constructor, and then does a check: if a field is non-null, it updates that field in the database, otherwise it doesn't.

Of course, every single one of those parameters is stringly typed, which makes it super fun. Tracking premiumAmount and terminationDate as strings is certainly never going to lead to problems. I especially like the pdfPassword being stored, which is clearly just the low-security password meant to be used for encrypting a transaction statement or similar: "the last 4 digits of your SSN" or whatever. So I guess it's okay that it's being stored in the clear in the database, but also I still hate it. Do better!

In any case, this function was called twice. Once from the form that Matt was editing, where every parameter was filled in. The second time, it was called like this:

int nbUpdates = incoming.saveQualif(docClass, null, null, null, null, null, multiDeal, null,
                null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null,
                null, null, null, null, null, null, null, null, null, null, null, null);

As tempted as Matt was to fix this method and break it up into multiple calls or change the parameters to a set of classes or anything better, he was too concerned about breaking something and spending a lot of time on something which was meant to be a small, fast task. So like everyone who'd come before him, he just slapped in another parameter, tested it, and called it a day.

Refactoring is a problem for tomorrow's developer.

Some time ago, poor Keith found himself working on an antique Classic ASP codebase. Classic ASP uses VBScript, which is like VisualBasic 6.0, but worse in most ways. That's not to say that VBScript code is automatically bad, but the language certainly doesn't help you write clean code.

In any case, the previous developer needed to make an 8 element array to store some data. Traditionally, in VBScript, you might declare it like so:

Dim params(8)

That's the easy, obvious way a normal developer might do it.

Keith's co-worker did this instead:

Dim params : params = Split(",,,,,,,", ",")

Yes, this creates an array using the Split function on a string of only commas. 7, to be exact. Which, when split, creates 8 empty substrings.

We make fun of stringly typed data a lot here, but this is an entirely new level of stringly typed initialization.

We can only hope that this code has finally been retired, but given that it was still in use well past the end-of-life for Classic ASP, it may continue to lurk out there, waiting for another hapless developer to stumble into its grasp.

Kerry (previously) has a long held belief: people that can’t get the little things right have no hope of getting the big things right, and not just when it comes to software. Personally, I don’t...

There's bad date handling code. There's bad date formatting code. There's bad date handling code that abuses date formatting to stringify dates. There are cases where the developer...