DevDisasters

Unlucky 13?

Would going above and beyond when fixing terrible code have serious consequences?

• By Mark Bowytz
• 03/12/2014

Evan (not real name) quickly claimed a seat near the middle of the long wood-grain conference room table, ensuring he was ahead of his other teammates.

His plan was to delay having to give his project update as long as possible; he figured no matter which end of the table the team lead started on, it could take a long time to reach him. In fact, there was always the chance that someone might filibuster on some irrelevant point and they'd have to postpone the meeting updates for another day.

Evan sipped at his bottle of water and sighed, feeling his internal pressure being relieved slightly ... but that feeling wasn't long-lived.

You have nothing to worry about, Evan tried to reassure himself. He could play it all off as new-hire jitters -- after all, he did only just start a few weeks ago as a developer for a large media company in Istanbul, Turkey. Unfortunately, his nervousness didn't have anything to do with being a new guy.

He was afraid he'd have to own up to fixing somebody's awful code ... without being asked to.

Radical Changes
It had to do with his first assignment after being hired: Evan was asked to make a change to one of the company's client's Web sites, a perfect "new guy," super-simple-type change. You know, get the hang of the promote process, source control layout and so on. The site owner had asked for the number of items in their tag cloud to be increased from 13 to 20. No special formatting changes, no extra work. Just increase a number from 13 to 20.

"It'll take you 5 minutes -- I swear," he was assured by his project lead.

After scrounging through some kludgy HTML and codebehind, Evan managed to find the stored procedure responsible for getting 13 random entries from the database.

He expected to find something simple and elegant. Something that can leverage built-in functionality. Something like:

select top(13) * from encyclopedia_cat order by newid()  

After all, this was not some tiny development shop. There were some real gurus at his company.

Alas, for some individuals, one line of code is not enough if you're taking your job seriously:

CREATE PROCEDURE [dbo].[P_encyclopedia_rnd] 
@recordcount INT=13
AS
DECLARE @value INT

CREATE TABLE #data
  (
    id INT
  )

DECLARE @amount INT

SELECT @amount = Count(1) FROM encyclopedia_cat

DECLARE @counter SMALLINT

SET @counter = 1

WHILE @counter < @recordcount
  BEGIN
    --select rand() random_number,convert(int,rand()*@amount) 
      as product, @amount 
    SET @value=CONVERT(INT, Rand() * @amount)

    IF NOT EXISTS(SELECT *
                  FROM #data
                  WHERE id = @value)
      BEGIN
        IF EXISTS(SELECT *
                  FROM encyclopedia_cat
                  WHERE catid = @value
                  AND Len(catname) < 40)
          BEGIN
            INSERT INTO #data
              (id)
            VALUES
              (@value)

            SET @counter = @counter + 1
          END
      END
  END

SELECT v2.*, LEFT(catname, 1) AS letter
  FROM #data v1
  JOIN encyclopedia_cat v2
    ON v1.id = v2.catid
    --where len(v2.catname)<40 

SELECT *
  FROM #data

DROP TABLE #data 

Evan couldn't figure how users weren't complaining about site performance -- full scan of the "encyclopedia" table, 13 SELECTs, abuse of Temp table space every time someone visits the page? What a train wreck. (How bad? See for yourself here.)

Having an efficient and, above all, sensible, one-line solution in mind, Evan replaced the existing code, tested and deployed his change. Excluding testing and waiting for the build processes to finish, elapsed time was close to the five minutes he was originally promised.

Evan was feeling pretty smug, having not only implemented the change he was asked to make, but also radically refactoring the original code, which wasn't in the scope of the change request.

Suddenly, Evan's mind started reeling. He felt as though his brain had been switched into overdrive: What if there was some valid reason for the code to work that way? Maybe I was being tested to see how well I could follow instructions? A myriad of possibilities all played out in his head. Just to be sure, he double-checked the live site in production -- yep, 20 tags as requested. But would anybody notice his radical change?

A True Confession
Sadly, the filibusters were kept to a minimum, and before Evan realized it, it was time to provide his status update. Knowing there was no hiding from the truth in the code, Evan spilled his guts: the code, the changes, the fact that he tested the changes thoroughly before promoting. Everything.

Evan expected to be chastised or, worse, find himself being advised about 'best practices' on the team and the consequences of introducing so much risk into the site's code. But that didn't happen -- instead, the team lead seemed to be quite ... pleased?

"Actually, I did notice your changes in the promote e-mails. We asked an intern to code that site a few months ago and the 'logic,' if you can call it that, has been a thorn in our side ever since and we were just too busy to fix it. Great work!"

Was Evan dreaming? Rewarded for doing the right thing? He wasn't quite sure what to think. It definitely wasn't like this at his previous job.

The team lead continued, "How'd you know to take the approach that you did?"

Still recovering from the shock to his senses, Evan couldn't remember the simple coding change he had made earlier and instead sheepishly replied, "Just got lucky, I guess!"