It was Brians first day at AutoDetective, a website for comparing listed car prices vs. blue book values. His work inbox was overflowing with style guides, best practices, and notes from the dozen or so other developers he would be working with. His interviewer, Douglas, had mentioned that the site ran on a substantial chunk of legacy code, but Brian had experience with plenty of old code.

He spent most of the day digging through the source, getting a feel for the in-house development style. It didnt take long before he noticed how & off the code was.

It wasnt just legacy code. It was obtuse legacy code.

Just Dont Touch It

Douglas came to debrief Brian at the end of the day. Brian explained how he spent the afternoon going through the codebase, looking for a project to get his hands wet. Theres a lot of inefficient code, he said. I figured Id fix something small. He pointed to a bit of code:

if (isset($_SESSION['relogin_data'])) {
 $a_relogin_data = $_SESSION['relogin_data'];
 $arr_keys = array_keys($_SESSION);
 for ($i=0; $i < sizeof($arr_keys); $i++) {
 unset($_SESSION[$arr_keys[$i]]);  
 }  
 $_SESSION['relogin_data'] = $a_relogin_data;
}

Why are we unsetting the entire array like this? Ive pared this down in my local branch. I was about to send a pull request

No, Douglas said. Thats our legacy code. We cant change a single line in that.

But its inefficient

It doesnt matter. None of us who still work here understand this code. Its like a house of cards: if we change something little, the whole thing could come crashing down.

Oh-kay. Brian quietly deleted his local branch.

Stovepipes Attached to Stovepipes

Every day, when he finished his assigned trouble tickets, Brian would attempt to make sense of the huge lump of legacy code that no one in the company could ever touch. For instance, this ultra-robust array building method, which skips simple assignation for laborious key/value reassignments:

while ($row = mysqli_fetch_assoc($result))
{
 $array_keys = array_keys($row);
 $array_values = array_values($row);

 for ($i=0; $i < sizeof($array_keys); $i++)
 $dataset[$count][$array_keys[$i]] = $array_values[$i];
 $count ++;
}

The code below checks if theres a connection already open with a given name. All database errors in the legacy codebase return false, so theres no way to tell if its a bad password, a server outage, or trying to reuse the same connection name twice:

if (isset(DBClass::$open_connections_[$conn_name])) {
 // connection is already open
 return false;
}

Everything, and one means everything, was stored in $_SESSION. Dumping session data was almost useless with how much was stored in it on a regular basis:

public function getUserInfo()
{
 return $_SESSION['logged_in'];
}
$user_info = $login->getUserInfo();
$_SESSION['user_info'] = $user_info;

// ...

$_SESSION['logged_in'] = $this->a_logged_in_;
$_SESSION['view_mode'] = $_SESSION['logged_in']['view_mode'];
$_SESSION['user_info'] = $_SESSION['logged_in'];

Then there was the utterly useless, such as a defined destructor that only calls its parent (unnecessary in PHP):

 public function __destruct()
 {
 parent::__destruct();
 }

A Unit Test Too Far

One afternoon, Brian willed himself to Douglass office. When his boss asked what the trouble was, Brian pointed him to a bit of code he had been staring at for hours. I was working on #75693, and trying to figure out where $this->SERIAL was getting set. It wasnt in the class definitions, which would be natural. Instead, I found it in this out-of-the-way include.

http://thedailywtf.com/articles/aged-like-vinegar