Last minute geek

last minute tech news from around the net

Tuesday, Oct 16th

Last update01:00:00 AM

You are here: English WTF CodeSOD: An Eventful Career Continues

CodeSOD: An Eventful Career Continues

User Rating: / 0
PoorBest 

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!

Read all
Comment Policy:
We pre-moderate any comments and welcome all kinds of thoughts, supportive, dissenting, critical or otherwise. We delete or censor comments that are:

* abusive
* off-topic
* contain personal attacks, or against any company or organization
* promote hate of any kind
* use excessively foul language
* is blatantly spam or advertising

We do not discriminate based on the person who is posting, and we never censor comments for political or ideological reasons. We never delete an appropriate comment because we disagree with its viewpoint or ideology, and we never publish an inappropriate comment because we agree with or support its viewpoint or ideology.


Attention spammers: we manually approve all comments. Spamming and blatant advertising will NOT be published on this site and is deleted immediately, you've been warned, do not waste your time here.

Add comment

Security code
Refresh