Theresa + CTO Code: Worlds collide

The_Quiet_One

Branching is bad, use feature toggling instead! reminded me of a story from the early years of my career. Back then I was working for a company with a multi-tenant web portal app that had a bunch of customers, big and small. It was my first job out of college, and on one of my first days while perusing their PHP codebase, I came across a few oddities.

$i = 0;

define("REPORT_COL_NAME", $i++);
define("REPORT_COL_DESC", $i++);
$i++;
define("REPORT_COL_TOTAL_SUM", $i++);
$i -= 2;
define("REPORT_COL_SUBTOTAL_SUM", $i);
$i += 3;

I asked someone about this particular... peculiarity, and the CTO said it wasn't his code, but the code of Theresa. Theresa's code was notorious for these kinds of things. And she had tons of psychological problems she was being medicated for, and I would say looking at her code was kind of like a window into her mind. It was fascinating. And irritating. I could best describe her code style as "stream of consciousness." It's as if she'd literally just type her logic as it came to her, and never even go back to refactor.

Then there were tons of places where I found stuff like the following:

if ($isITC) {
    // do something
}
else {
    // do something slightly different
}

This was peppered throughout the codebase. Everywhere. I asked the CTO "What does isITC mean?"

"is Ini-Tech Corp. It's a global variable."

"Oh, more Theresa code?"

He looked a little insulted. "No, it's mine."

I recognized Ini-Tech as one of our premiere customers. Later on, I saw other similar global variables: $isAcme, $isEdiabitzocity59line, etc. All forked in their codebase.

I was entry-level, but I still felt a little sick to my stomach seeing this. But the hurl didn't come about until I saw what happens when CTO code merges with Theresa code. It's a result I think would cause detectable gravitational waves:

$i = 0;

define("REPORT_COL_ID", $i++);
define("REPORT_COL_BNAME", $i++);
if ($isITC) {
    $i += 5;
}
else {
    $i++;
}
define("REPORT_COL_BDATE", $i++);
if ($isITC) {
    $i -= 4;
}
else if ($isAcme) {
    $i += 2;
}
else {
    $i++;
}
define("REPORT_COL_BDELETED", $i++);
if ($isAcme) {
    $i--;
}
else {
    $i++;
}

I'm giving you the worst of it. Fortunately Theresa code wasn't everywhere. She actually had only a few places she really had "code ownership" over. The problem was, she eventually did have a mental breakdown that prevented her from doing any more work, and as a result I adopted her codebase. Which is part of the reason it might have been my first job but it certainly isn't my current one.

Tsaukpaetra

@The_Quiet_One said in Theresa + CTO Code: Worlds collide:

define("REPORT_COL_NAME", $i++);

You know, I was considering converting our columnar definitions to index-based:

0_1541123896844_762421dd-6e8c-4e39-a258-912ea934f26e-image.png

But now I see the wisdom of using an self-incrementing number!

BernieTheBernie

@The_Quiet_One said in Theresa + CTO Code: Worlds collide:

define("REPORT_COL_TOTAL_SUM", $i++);

Did she use $i++ and $i only in the define function? Hey, using some ++$i could enhance confusion even more.

define("REPORT_COL_WTF_FRIST", $i++); define("REPORT_COL_WTF_SECNOD", $i); define("REPORT_COL_WTF_LSAT", ++$i);

ixvedeusi

@BernieTheBernie said in Theresa + CTO Code: Worlds collide:

enhance confusion

The_Quiet_One

@Tsaukpaetra the worst part is when I'd have to debug (we didn't have any nice debugging ide for PHP) we'd just print the value and it'd end up being a number so I'd have to go back to the code and figure out what constant it was. There was at least one time a bug was narrowed down to having two constants assigned to the same number, but only if the customer was Ini-Tech Corp. One of the first thing I did was replace that with assigning those constants to actual number values.

BernieTheBernie

@ixvedeusi said in Theresa + CTO Code: Worlds collide:

@BernieTheBernie said in Theresa + CTO Code: Worlds collide:

enhance confusion

Well, that's the sole purpose of prefix or postfix operators on parameters.

izzion

@BernieTheBernie
Said like a small-minded individual who's not capable of understanding the glorious LOC savings in using prefix & postfix operators.

BernieTheBernie

@izzion Yeah, I know: the more lines of code, the slower the program runs. Well, make it working best for you: when you write your frist version, do it verbose with many additional lines (because here you are paid by LOC), and when the program is too slow, enhance performance by putting everything into a single line.