Thursday, 27 March 2008

Code horror

Here's something that was brought to my attention this evening, when I should be working on encounters for a Friday night WFRP game... no, instead I'm working. Literally. On a production problem. For my employer.

Scenario: a badly written stored procedure has a race condition. Instead of fixing the stored procedure, get the web geeks to fix it on the front end with Javascript. Problem: double clicking a submit button inserts duplicate records in the backend. Solution: stop users from double clicking, because fixing the database or the stored procedure is not the place you want to fix this problem. Whatever.

Assignment: get a flunky in a remote production support facility (we haven't left the US, fortunately) to craft a Javascript function that would prevent the form from submitting twice.

var submitted=0;
function checkAlreadyDone() {
        if (submitted < 2) {
                document.forms[0 ].submit();

There are a couple of problems with this charlie foxtrot. The first being the zero-indexed variable and the boolean check for less than 2! WTF? is that. Why not a boolean? It's easier to read. Now I have to do math to review the code. It's true or false! The second is I have think about what the variable state is on the round trip of the submission. 0, now 1, or less than 2. It changes to often.

I wanted to cry. And not just because the really broken code: the stored procedure and the database aren't going to be corrected, but because this is just badly written. The flunky never even requested a code review, never tested it because the call to the forms array was submitting the wrong form and creating a Javascript error in the browser! and basically broke the functionality of the page for over 6 million users.

Posted by caffeinated at 10:37 PM in 0xDECAF


[Trackback URL for this entry]

Your comment:

(not displayed)

Live Comment Preview:

« March »