Cube with a blocked view.

Drowning in bad coding practices

Well, I'd hoped I wouldn't be back to comment on this, but here I am. These past two weeks, I've been tasked with revising a major application in our corporation. The problems I came across earlier were annoying and made code maintenance difficult. The problems I encountered yesterday and today have the same qualities - annoying, difficult maintenance... with an added bonus. They're funny.

I'll admit maybe they're only funny to me because I'm at that point working with the code. Or maybe it has something to do with the downright giddy mood I've been in for the last couple days. Maybe it's because someone might actually buy my house at some point in the near or distant future. Maybe it's just that sense of absolute futility finally setting in. Regardless, here I am with an application with so many design and coding rules not just broken, but smashed and walked all over, that I can't help but giggle.

General overview of the system - it's used to request stuff, which means it's got a large amount of workflow surrounding it. Couple levels of approvals, emails sent, links sent, etc. You know the drill. Oh, one other thing...for all you RDBMS guys and girls think of a document being like a record in your world. Similar concept. A view is like a grid for you guys displaying records.

One task I was assigned was allowing a specific group access to the database to change the approver for a document. So I did this. I changed the approver, any approver information stored in the document, the status, and gave the new approver proper access to the document. Managed to get the code working changed a couple approvers to myself and take a peek at it on the web... hmmm, these documents aren't showing up in My Approvals. Interesting. So off I run to check the documents again, yup, status changed, my name set as the approver... Odd. I open that web view in designer and take a look at the view selection. Hey, wait a second... it's using a different status field than I set. Odd...

By the time I finished poking around, I ended up having to set no fewer than THREE status fields in my code. THREE. All of which are used to track the same thing - the status of the request through the approval process. THREE fields, THREE columns in a table, used for the same purpose. Sigh.

At the same time, while giving these users a pretty little button to change the approver and resubmit the request, need to resend an email to the new approver. So off I go to steal the code from the agents that were created originally for this purpose. These are the same pieces of code that gave me nightmares last week with their bad coding practices. But, optimistic as ever, off I run to steal the email code from there (yup, in one of those pretty gosubs). Copy and paste and low and behold...an error. Something not declared. By the time I'm finished yanking the teeth that are variables used throughout the creation of the mail message, I learn something.

Whoever wrote this code didn't understand variables and scope. This is where I began to laugh the laugh of the insane. They declared several global variables (Even now I can't keep a straight face) for a chunk of code that is essentially all at the same level. But wait...one of the actual subroutines written is written to just set some of the global variables. Yup. You read that right. A subroutine (not one of those gosub dealies) written to set global variables that are used in the main body of the code and do not need to be global.

It gets better. Another of these global variables is used in the one other subroutine written outside the main body of code. This subroutine essentially get's a value and then sets one of the global variables with the value. To be used...yup, you guessed it...in the main body of the code. What's funny here is that a global variable didn't need to be used if they'd just made the subroutine a function that returns a string. One less variable. Oh wait, if they did that, and realized that the little "initialize objects" routine is pointless...the agent would just be one long badly written, spaghetti code, subroutine embedded in subroutine, chunk of code. And I wouldn't have gotten the laugh out of it that I did.

But hey, as I wait to find out how my brother's surgery went yesterday, it's good to have something inane to laugh about.

posted by Erin @ 9:30 AM