CodeSOD: An Eventful Career Continues

You may remember Sandra from her rather inglorious start at Initrovent. She didn't intend to continue working for Karl for very long, but she also didn't run out the door screaming. Perhaps she should have, but if she had- we wouldn't have this code.

Initrovent was an event-planning company, and thus needed to manage events, shows, and spaces. They wrote their own exotic suite of software to manage that task.

This code predates their current source control system, and thus it lacks any way to blame the person responsible. Karl, however, was happy to point out that he used to do Sandra's job, and he knew a thing or two about programming. "My fingerprints are on pretty much every line of code," he was proud to say.

if($showType == 'unassigned' || $showType == 'unassigned' || $showType == 'new') { ...}

For a taster, here's one that just leaves me puzzling. Were it a long list, I could more easily see how the same value might appear multiple times. A thirty line conditional would be more of a WTF, but I can at least understand it. There are only three options, two of them are duplicates, and they're right next to each other.

What if you wanted to conditionally enable debugging messages. Try this approach on for size.

foreach($current_open as $key => $value) { if ($value['HostOrganization']['ticket_reference'] == '400220') { //debug($value); }}

What a lovely use of magic numbers. I also like the mix of PascalCase and snake_case keys. But see, if there's any unfilled reservation for a ticket reference number of 400220, we'll print out a debugging message" if the debug statement isn't commented out, anyway.

With that in mind, let's think about a real-world problem. For a certain set of events, you don't want to send emails to the client. The planner wants to send those emails manually. Who knows why? It doesn't matter. This would be a trivial task, yes? Simply chuck a flag on the database table- manual_emails and add a code branch. You could do that, yes, but remember how we controlled the printing of debugging messages before. You know how they actually did this:

$hackSkipEventIds = array('55084514-0864-46b6-95aa-6748525ee4db');if (in_array($eventId, $hackSkipEventIds)) { // Before we implement #<redacted>, we prefer to skip all roommate // notifications in certain events, and just let the planner send // manual emails. return;}

Look how extensible this solution is- if you ever need to disable emails for more events, you can just extend this array. There's no need to add a UI or anything!

[Advertisement] Utilize BuildMaster to release your software with confidence, at the pace your business demands. Download today!