CodeSOD: A Mid Query

Many years ago, Tom supported a VB6 application. It was about 750,000 lines of code, split across far too many files, with no real organization to it. While the code was bad and awful, the organization problem was a universal issue for the application. It even infested the database.

This is some VB6 code for querying that database:

strSQL = “SELECT * FROM ________ WHERE “ & _
“project_num = ‘” & rsOpening(“project_num”) & “‘ AND” & _
“[manf_abbr] = ‘” & rsDoorpodet!manf_abbr & “‘ AND “ & _
“[series] = ‘” & Mid(rsDoorpodet!SeriesSize, 1, 10) & “‘ AND “ & _
“[size] = ‘” & Mid(rsDoorpodet!SeriesSize, 11, 28) & “‘ AND “ & _
“[material_and_gauge] = ‘” & Mid(rsDoorpodet!material_and_gauge_and_finish, 1, 22) & “‘ AND “ & _
“[finish_and_core] = ‘” & g_current_finish & Mid(rsDoorpodet!core_and_label_and_hand, 1, 20) & “‘ AND “ & _
“[label_and_machining_code] = ‘” & g_current_label & Mid(rsDoorpodet!machining_code_and_undercut, 1, 30) & “‘ AND “ & _
“[undercut_and_seamless_and_door_type] = ‘” & g_current_undercut & Mid(rsDoorpodet!SeamlessDoorTypeElevPricegroupPrepgroup,1,11)&“‘AND” & _
“[ElevPrepGroup] = ‘” & Mid(rsDoorpodet!SeamlessDoorTypeElevPricegroupPrepgroup, 12, 8) & Mid(rsDoorpodet!SeamlessDoorTypeElevPricegroupPrepgroup, 21, 5) & “‘ AND “ & _
“[WdDrAttrKey] = “ & rsDoorpodet!WdDrAttrKey

The Hungarian notation tells us that this query is driven by the results of another query- the rs prefix on rsOpening and rsDoorpodet tell us that much. It makes you wonder if maybe this should be a join in the database instead, but then we notice the pile of Mids in the where clause.

Mid(rsDoorpodet!SeriesSize, 1, 10) and Mid(rsDoorpodet!SeriesSize, 11, 28)- the field SeriesSize holds two values: series and size. I believe “storing data as concatenated fields” is the -1th Normal Form for database normalization. And let’s not miss SeamlessDoorTypeElevPricegroupPrepgroup, which I’m not even sure what all the components in that mean.

But we even end up getting confused- material_and_gauge_and_finish gets parsed for material_and_gauge, but when we want to populate finish_and_core we get the finish from g_current_finish- a global variable (based on Hungarian notation), and a lovely little SQL injection attack waiting to happen.

I suspect that this database schema was driven by the tools used to manage this data before there was a database- probably somebody’s Excel spreadsheet. Instead of normalizing the data, they just blindly converted a spreadsheet into an application, and this was the result.

Source: Read More