It’s An Ill Wind

I was pontificating recently about the horror of tightly-coupled worksheet calculations and macro code in Excel and the mess that results from carelessly mashed-together programming paradigms.

By a horrible coincidence, a few smug weeks later I suddenly found myself deep in the horror again. This time, I had to contend with two versions, needing to set up and run several tens of thousands of real-world cases through “before” and “after” scenarios.

I soon started to turn up nuggets of wonderment. Here are a couple of gems from the worksheets:

=IF(CPPI=TRUE,TRUE,FALSE)
=MAX(1,DATE(YEAR(C4),MONTH(C4),DAY(C4))-DATE(YEAR(C6),MONTH(C6),DAY(C6)))

“Why”, I asked myself, “don’t these monkeys think?” (When someone asked me which monkeys I was referring to, I realised that my inner monologue was malfunctioning, but that’s not important right now.)

Obviously they were thinking, apart from anything else the whole thing does seem to work, inasmuch as a set of inputs produce a set of plausible-looking outputs. I’d assert, however, that an experienced, competent developer has a continuous, higher-order review process running, asking questions like “does that look right?”, “could that be written better?”, “is that clear?”, “could someone else understand that?” It would appear that the individuals concerned in this case had no such process running.

I have a little tool that I bring out when faced with an unknown Excel VBA application. It scans the code and delives a report that gives me an idea of the degree of caution I should exercise, calculating for each routine an “X” number, being some arcane function of code lines, mesting level, numbers of ifs, loops, one-character variables (seldom straightforward to rename, even if they’re called “q” or “z”). A score of 10 or above indicates a possible problem area. Over 50 and we’re definitely in bandit country. Here’s the report for the top-scoring routine in the horror workbook (names changed to conceal the guilty):

422 is by a factor of about 3 the highest-scoring routine I’ve ever encountered. (By contrast, and perhaps it’s an unfair comparison, the top score in xlunit is 24.) That 1044 lines of code doesn’t include comments, blank lines, variable declarations or similar. Even better, it’s declared Static, something so appallingly dangerous (to me, at least – I get nauseous when I see even a Static variable) that I had to change the analyser because it had never seen it before. “Option Explicit” is honoured more in the breach than the observance and there are a mere (ha!) 156 global variables.

I was flicking through my cherished (and slightly stained) copy of The Pragmatic Programmer last night and I’ll take as my text the section “What Make a Pragmatic Programmer?” on pages xiii and xix of the Preface. Summarising the characteristics and comparing them with the authors of the unholy mess described, I get:

Early adopter/fast adapter: given that the abomination I’m working with is the culmination of more than two years’ work and is the fourth or fifth (and probably “best”) version, FAIL.

Inquisitive: I see no sign of any curiosity, not even “why do we suck?” FAIL

Realistic: I’m (generously) prepared to specify that this one may not really apply here. (Big of me.)

Jack of all trades: and masters of none? These guys didn’t make it to Jack of one trade. FAIL.

Critical Thinker: referring to the inner monologue again, there’s no evidence to suggest that anything like the questions listed above were ever applied here. And plenty of evidence to the contrary. BIG RED FAIL.

The tragedy here is that the monkeys described are unlikely to realise exactly how snivellingly far from acceptable (let’s not waste “best” here) practice they are, as the Dunning-Kruger effect tells us is likely, so they’re not going to change in any useful way. Failing to see that the twin demons of high coupling and low (non-existent, if we’re honest) cohesion are rampaging through their code leaves me wondering where we’d start.

Depressingly, these guys aren’t unusual, they’re not even the worst. The type is perhaps a little more common in, say, the Excel world, where the tools to develop staggering incompetence are so widely available, but they’re everywhere else – the invisible 90% of the iceberg.

On the plus side, even moderately competent technicians who are prepared to don the elbow-length protective gloves and dive in to such messes will be able to look forward with some confidence to a lifetime in which there will be a paycheck for as long as they can stand to pick it up.

The Glove Puppet Programmer

While I prevaricate over the four (!) drafts sitting at various stages of incompletion in my “Posts” view, I’ll take lunchtime out to reminisce about a non-contemporaneous* colleague and his unique status as probably the most expensive programmer I’ve ever had to deal with.

This particular individual, who was either French or French Canadian, had the singular identifying characteristic of a name that was a literal translation of a stuffed children’s TV character from my childhood. This in itself should have been a warning. He had created the Credit Risk reporting system that a team of ten expensive contractors, myself including, were labouring feverishly to replace. On his own. As a series of Excel spreadsheets. With extensive VBA macros, coming from no discernible programming background. If you’re wincing, I’m not surprised.

Why am I asserting he was peerless in the cost stakes? Because the application he’d built actually worked. Badly, slowly, requiring regular manual intervention, occasionally obviously inaccurate and more frequently rather less so, but it worked. And we were trying to reverse-engineer it**, distinguish between the right and wrong parts and deliver something that was everything the old “system” was not. Well, almost: we did need our system to work.

This is one of the gems I discovered in the VBA:

    br_OutputWB.Worksheets(bv_sOutPutWSName).Range("A1").offset(br_lCount + 1, 0).value = bv_sSTRATEGY
    br_OutputWB.Worksheets(bv_sOutPutWSName).Range("A1").offset(br_lCount + 1, 1).value = bv_sTRADENAME
    br_OutputWB.Worksheets(bv_sOutPutWSName).Range("A1").offset(br_lCount + 1, 2).value = bv_sTRADESTATUS
    br_OutputWB.Worksheets(bv_sOutPutWSName).Range("A1").offset(br_lCount + 1, 3).value = bv_sStructType
    br_OutputWB.Worksheets(bv_sOutPutWSName).Range("A1").offset(br_lCount + 1, 4).value = bv_sSTRUCTMODEL
    br_OutputWB.Worksheets(bv_sOutPutWSName).Range("A1").offset(br_lCount + 1, 5).value = bv_sTRANCHENAME
    br_OutputWB.Worksheets(bv_sOutPutWSName).Range("A1").offset(br_lCount + 1, 6).value = bv_iTRANCHENUMBER
    br_OutputWB.Worksheets(bv_sOutPutWSName).Range("A1").offset(br_lCount + 1, 7).value = bv_lTRANCHENOTIONAL

    ... are you getting the picture? We continue incrementing the offset for a while, until:

    br_OutputWB.Worksheets(bv_sOutPutWSName).Range("A1").offset(br_lCount + 1, 29).value = bv_sCOMMENTDEFAULT
    br_OutputWB.Worksheets(bv_sOutPutWSName).Range("A1").offset(br_lCount + 1, 30).value = bv_sNEWTRANSID
    br_OutputWB.Worksheets(bv_sOutPutWSName).Range("A1").offset(br_lCount + 1, 31).value = bv_sCOMMENT

In case you were wondering, the “bv_” prefix signifies “By Value.” And yes, that does mean that each of those values appeared as parameter to the routine. I counted 32 in total. That’s thirty-two input parameters. You really should be wincing by now.

Several other routines also had the same (or 99% the same) code blocks. Our friend the cut-and-paste programmer must have really earned his money that day…

Oh yes, you should see an example of how our chum called his parameter-heavy subroutines. Consider acquiring welding goggles before looking at this, by the way.

Call SS_BuildMTMByTrade(br_OutputWB:=OutputWB, br_lCount:=lMMcount_SS, bv_dbFXrate:=GetFXRate(vMM_FXRates, RgCursor.offset(lOffset, Range("TRANCHECCY").Column - 1).value), _
bv_sSourceFile:=m_sSS_MM_Source & IIf(bv_sTradeNamePrefix <> "", bv_sTradeNamePrefix & RgCursor.offset(lOffset, Range("TRADENAME").Column - 1).value, RgCursor.offset(lOffset, Range("TRADENAME").Column - 1).value) & "_" & "MM" & "_" & RgCursor.offset(lOffset, Range("MM_CSB").Column - 1).value & "_" & RgCursor.offset(lOffset, Range("MM_R").Column - 1).value & "_" & IIf(RgCursor.offset(lOffset, Range("MM_PW").Column - 1).value, "T", "F") & "_" & IIf(RgCursor.offset(lOffset, Range("MM_SCS").Column - 1).value, "T", "F") & "_" & Format(m_dSS_MM_Asof, "yyyymmdd") & ".xls", bv_dbParticipationFactor:=RgCursor.offset(lOffset, Range("DEALNOTIONAL").Column - 1).value / RgCursor.offset(lOffset, Range("TRANCHENOTIONAL").Column - 1).value, bv_iShift:=RgCursor.offset(lOffset, Range("MM_R").Column - 1).value, bv_bIgnoreFirstCprty:=CBool(RgCursor.offset(lOffset, Range("IGNORECPTY").Column - 1).value), _
bv_dbSpread:=RgCursor.offset(lOffset, Range("DEALSPREADBP").Column - 1).value, _
bv_bUsePVFormula:=RgCursor.offset(lOffset, Range("USEPVFORMULA").Column - 1).value, _
bv_sSTRATEGY:=RgCursor.offset(lOffset, Range("STRATEGY").Column - 1).value, _
bv_sTRADENAME:=IIf(bv_sTradeNamePrefix <> "", bv_sTradeNamePrefix & RgCursor.offset(lOffset, Range("TRADENAME").Column - 1).value, RgCursor.offset(lOffset, Range("TRADENAME").Column - 1).value), _
bv_sTRADESTATUS:=RgCursor.offset(lOffset, Range("TRADESTATUS").Column - 1).value, _
bv_sStructType:=RgCursor.offset(lOffset, Range("STRUCTTYPE").Column - 1).value, bv_sSTRUCTMODEL:=RgCursor.offset(lOffset, Range("STRUCTMODEL").Column - 1).value, _
bv_sTRANCHENAME:=RgCursor.offset(lOffset, Range("TRANCHENAME").Column - 1).value, bv_iTRANCHENUMBER:=RgCursor.offset(lOffset, Range("TRANCHENUMBER").Column - 1).value, bv_lTRANCHENOTIONAL:=RgCursor.offset(lOffset, Range("TRANCHENOTIONAL").Column - 1).value, bv_sTRANCHESUBORDINATION:=RgCursor.offset(lOffset, Range("TRANCHESUBORDINATION").Column - 1).value, bv_sTRANCHECCY:=RgCursor.offset(lOffset, Range("TRANCHECCY").Column - 1).value, bv_dtTRANCHEMATURITY:=RgCursor.offset(lOffset, Range("TRANCHEMATURITY").Column - 1).value, _
bv_sBUYSELL:=RgCursor.offset(lOffset, Range("BUYSELL").Column - 1).value, _
bv_lDEALNOTIONAL:=RgCursor.offset(lOffset, Range("DEALNOTIONAL").Column - 1).value, _
bv_iDEALSPREADBP:=RgCursor.offset(lOffset, Range("DEALSPREADBP").Column - 1).value, _
bv_sUPFRONTFEE:=RgCursor.offset(lOffset, Range("UPFRONTFEE").Column - 1).value, _
bv_sCOUNTERPARTY:=RgCursor.offset(lOffset, Range("COUNTERPARTY").Column - 1).value, _
bv_dtTRADEDATE:=RgCursor.offset(lOffset, Range("TRADEDATE").Column - 1).value, _
bv_dtSETTLMTDATE:=RgCursor.offset(lOffset, Range("SETTLMTDATE").Column - 1).value, _
bv_sREPACKVEHICLE:=RgCursor.offset(lOffset, Range("REPACKVEHICLE").Column - 1).value, _
bv_sCOMMENT:=RgCursor.offset(lOffset, Range("COMMENT").Column - 1).value, _
bv_lADRNOTIONAL:=RgCursor.offset(lOffset, Range("ADRN").Column - 1).value, _
bv_sCOMMENTDEFAULT:=RgCursor.offset(lOffset, Range("COMMENTDEFAULT").Column - 1).value, _
bv_sNEWTRANSID:=RgCursor.offset(lOffset, Range("NEWTRANSID").Column - 1).value, _
bv_sOutPutWSName:=MMTEMPSHEETNAME, _
bv_BookName:=RgCursor.offset(lOffset, Range("BOOKNAME").Column - 1).value, bv_UniqueID:=RgCursor.offset(lOffset, Range("UNIQUEID").Column - 1).value, bv_KMVCorr:=RgCursor.offset(lOffset, Range("KMVCORR").Column - 1).value, bv_MarketCorr:=RgCursor.offset(lOffset, Range("MARKETCORR").Column - 1).value, bv_RunFlag:=RgCursor.offset(lOffset, Range("RUNLISTFLAG").Column - 1).value, bv_DataSource:=RgCursor.offset(lOffset, Range("SOURCETYPE").Column - 1).value)

The second line is about 900 characters long. Nine. Hundred. If you’re starting to want to make it go away, perhaps by clawing out your eyeballs, I don’t blame you.

Finally, for dessert, if you will, or maybe like one of those hot steamy towels presented at the end of some Indian meals, here’s our friend’s take on copying an array:

Public Sub CopyArray(ByVal bv_FromArray As Variant, ByRef br_ToArray() As Variant)
    Dim l As Long
    Dim i As Integer

    For l = LBound(bv_FromArray, 2) To UBound(bv_FromArray, 2)
        ReDim Preserve br_ToArray(1, l)
        For i = LBound(bv_FromArray, 1) To UBound(bv_FromArray, 1)
        Debug.Print bv_FromArray(i, l)

            br_ToArray(i, l) = bv_FromArray(i, l)
        Next
    Next

End Sub

From the blank line following the “Debug.Print” line I surmise that some operation may, in the distant past, have been occurring that made the routine more than a criminal waste of electricity. I hope the use of lower-case “L” is a soothing balm to your eyes, as is the continuous “ReDim Preserve” of the outer loop…

I don’t know the final cost of the project that replaced this, I elected not to renew my contract after nine months. I’d guess it didn’t exceed $10 million, not counting the possible losses incurred from trading on inaccurate information in the several years the “system” was live.

I’d call that dangerous.

---

*we worked at the same place but not at the same time, if that makes any sense

** specs? Don’t be ridiculous.