Thursday, February 22, 2007

The Amazing Disappearing RecordSet

We have a client which uses a WBT Course Management System which they purchased. It is a classic ASP web application, whose standard database is Access and they have just recently introduced the ability to use SQL Server as a backend. Our task in this thing is hosting and fixing any issues which the company stumbles across while using it.

Not that big of a deal, one would think.

One would think.

Frankly, I'm not certain how this piece of software managed to make it out of Integration Testing. My guess, the developers don't know what integration testing is, let alone system or qualification testing.

Our client discovered an error, and asked us to find and fix it. So here we are, hunting through the code trying to figure out what is happening. Ultimately we discover that the problem is an array is not being populated from the database. No big deal. So we look closer, and notice that the array is being redim'd using the variable Num_of_Questions. Which makes sense. You want this array to be of a size of the number of questions you have. But during population, you have the index being referenced via the RecordSet field Perm_Question_Number.

Being a proponent of normalized databases, this through off warning bells in my head. For I assumed that it tied the question being asked to the answer.

Boy was I wrong.

I open up the database, and was greeted with a massive jumble of tables and fields. Consider the temporary table for holding user values:

Temp_User_Answers

  • User ID
  • Course_Number
  • Test_Number
  • Perm_Question_Number
  • Question
  • Answer A
  • Answer B
  • Answer C
  • Answer D
  • Answer E
  • Answer F
  • Answer G
  • Correct_Answer
  • Stud_Answer
Yeah. It tied to the Test_Content table via the Perm_Question_Number which was a one-based index for that Test's questions rather than the table's ID field (which was not set as a primary key, despite being the auto-incrementing, identity inserted field). And in case you're wondering, yes all of those answers and the question were repeated in that table as well.

Banging my head on my desk by this time, we get in contact with the developers of the software to ask them what they were smoking/drinking/etc while designing this software. Or if they even did design it. Or why they would do such a thing to a poor database.

Their response: "Ease of use."

If only, one could be throttled over the long-distance communication tools.

So, realizing that there was no help forthcoming from this crap-design as feature developers, we once more dug into the code trying to figure out why the values would disappear from the recordset.

Ultimately, we discovered that it has something to do with populating that array directly from the recordset. What was in the code was:
studAnswers(rRS("Perm_Question_Num")) = rRS("Stud_Answer")
correctAnswers(rRS("Perm_Question_Num")) = rRS("Correct_Answer")
And for whatever reason, this just refused to work. The system despised that line of code and refused to use the recordset after it (i.e. studAnswer was populated, but correctAnswers wasn't and any use of the recordset after it just acted like it didn't happen).

Well, having already discovered that I hate this code, and that it is riddled with flawed logic, shotty design, subpar commenting and no tabs, I figured why don't we just bandage it.

So we changed the code to look like this:
questionNumber = rRS("Perm_Question_Num")
studentAnswer = rRS("Stud_Answer")
rightAnswer = rRS("Correct_Answer")
studAnswer(questionNumber") = studentAnswer
correctAnswers("questionNumber") = rightAnswer
Lo, and behold, the page built successfully and properly.

Not the best solution, and I still don't know why the recordset would just stop working, but the application is running - and for our purposes that's what is important here. Anyone else that has the misfortune of buying this software can figure it out on their own (or hire us to do it).

So, with a bandage in place, I begin the process of removing the various Response.Write's which I had sprinkled throughout the page and add in a comment up at the top indicating that you know... a developer had done something here when I discovered the most perfect piece of code logic I have ever seen.

The one and only bit of error handling I found on the page.
If err.Number = 0 then
End If ' Check to see if there are no errors
Yes. You read that right. And yes, that was all there was to it. The developer, in his infinite coding goodness, took the time out to check the ADODB error object to see if it had no errors. He asked the code, if everything was right.

That echoing sound you hear, is probably me still smacking my head against the desk. The junior developer I was assisting in this maintenance issue is still chuckling over in his office. And we took the extra effort of going out and gathering the other developers and showing them this beautiful piece of logic.

Why he would check for success is beyond me. Frankly, in those scenarios I'm more worried about when err.Number is not equal to 0 as that means there is a problem which I need to deal with. If there's no problem, then I don't even need to spend time checking to see if there's no problem.

I'm flabbergasted at such coding. The entire thing gives all programmers a bad name.

No comments:

Blog Widget by LinkWithin