Keep on Searching



  •  I don't know when this code was written, but it was last worked on in 2011-- long after someone should have known better:

    @Okay so far said:


            sSQL = "..." // A fairly complex and db heavy search query

            oCommand = New OleDbCommand(sSQL, oConn)

            oReader = oCommand.ExecuteReader()

    Okay, so far so good. Execute a query, get back an OleDBReader so we can examine the data. It could go into a Dataset or a more modern object, but sure. Let's go with this. But then...

    @I'm getting a bad feeling said:

            dpth = 0

            While oReader.Read()
                dpth = dpth + 1
            End While

     Sure-- that does get the count of records returned. In the same way murdering your family members one by one will tell you how many family members you have. Had. Whatever. Maybe that's what Michael Myers was all about. He was just trying to count!

    Well, we've accomplished our mission. We know how many records this VERY DATABASE HEAVY SEARCH returns. I guess we can just close the reader, pack up and go home and-- wait, what's this? There's more?

    @HOLY FUCKING SHIT ARE YOU KIDDING ME? said:

                oReader.Close()

    If dpth > 0 Then

               oReader = oCommand.ExecuteReader()
                While oReader.Read() 

                             ... Render HTML result table ...
                End While

    End If

    Are you goddamn motherfucking kidding me?  Even if I were to accept the reality that this code was written before DataSet and Gridview were introduced into .Net (which it wasn't, and I reject that reality)-- and even if the dev had some fetish for manually looping over a dataset and hand-rendering the HTML (much in the same way some people have a sexual fetish for changing the diapers of physically deformed midgets with autism)-- even if

    HOW IN THE HOLY FUCK  did they think it would be a good idea to run the same query twice, rather than just using .HasRows?  Or put dpth into the While oReader.Read()? Or-- anything except running the query twice?

    Fucking facepalm moment of the month for me.



  • Copy-pasta code?



  •  more like Crappy-pasta



  • Yeesh. Depending on what gets rendered in the HTML table, they probably wouldn't even need the .HasRows check. If the query returns 0 results, then the table doesn't get populated. Much like if dpth = 0, the goddamn table doesn't get populated. There is no reason to count up the results first.

    PS- is it sad that I'm stuck on the fact this is obviously VB and in your first code chunk you added a C++ style comment?



  • Reminds me of code I wrote in my Intro to OOP course when we did file I/O. Which I knew was fucking garbage even when I wrote it, I just didn't know a better way to do it.



  • @Manni_reloaded said:

    PS- is it sad that I'm stuck on the fact this is obviously VB and in your first code chunk you added a C++ style comment?
    No mention of the super-crappy version of hungarian notation?



  • @Sutherlands said:

    @Manni_reloaded said:
    PS- is it sad that I'm stuck on the fact this is obviously VB and in your first code chunk you added a C++ style comment?
    No mention of the super-crappy version of hungarian notation?

    oReader, myReader...



  • You want to talk about shitty Hungarian notation? Here's an actual snippet from on the project I've inherited:


    [code]
    SqlConnection objConnection44 = new SqlConnection(ConfigurationManager.ConnectionStrings["XXXXXXXXXXX"].ConnectionString);



    //open the connection to the database

    objConnection44.Open();



    //create a new command using the connection object and select statement

    SqlCommand objCommand44 = new SqlCommand(strSqlStatement44, objConnection44);

    objCommand44.Parameters.Add("@registrant_entity_id", SqlDbType.Int).Value = strTempEntityId;

    objCommand44.Parameters.Add("@event_entity_id", SqlDbType.Int).Value = ViewState["intEventId"].ToString();

    objCommand44.Parameters.Add("@datestamp", SqlDbType.DateTime).Value = DateTime.Now.ToString();

    objCommand44.Parameters.Add("@source_name", SqlDbType.VarChar, 50).Value = objRblHearAboutUs.SelectedValue;

    objCommand44.Parameters.Add("@other", SqlDbType.VarChar, 255).Value = objTxbHearOther.Text;



    objCommand44.ExecuteNonQuery();



    //close the connection to the database

    objConnection44.Close();

    [/code]



    Those are all real variable names in our code base. "Yo dawg, I heard to like Hungarian notation, so I pre-pended obj to your rbl so you can facepalm while you facepalm."


  • Winner of the 2016 Presidential Election

    You know, the obj doesn't bother me nearly as much as the 44.



  • It just gets better from there. The WFTs abound. There's also [code] string strSqlStatement44. strSqlStatement [/code] (the original) first shows up on line 41. [code]strSqlStatement44[/code] shows up on line 707. The entire class is over 2000 lines of this shit.



  • @Lorne Kates said:

    @HOLY FUCKING SHIT ARE YOU KIDDING ME? said:

                oReader.Close()

    If dpth > 0 Then

               oReader = oCommand.ExecuteReader()
                While oReader.Read() 

                             ... Render HTML result table ...
                End While

    End If

    HOW IN THE HOLY FUCK  did they think it would be a good idea to run the same query twice, rather than just using .HasRows?  Or put dpth into the While oReader.Read()? Or-- anything except running the query twice?

    And presumably the db isn't locked between those two queries, so the second one could in fact return more or less rows than the dpth that was just counted?



  • @DaveK said:

    And presumably the db isn't locked between those two queries, so the second one could in fact return more or less rows than the dpth that was just counted?

    // Ensure number of rows doesn't change between the 2 executions of the query

    sSQL = "ALTER DATABASE xxxx SET READ_ONLY

    Of course, the Real WTF is that the record set should have been counted by the database by looping through the results with a cursor



  • @Lorne Kates said:

    Maybe that's what Michael Myers was all about. He was just trying to count!

    I've never actually seen Halloween so whenever someone mentions Michael Myers this is who I see:

    [Mike Myers as Austin Powers]



  • @Manni_reloaded said:

    PS- is it sad that I'm stuck on the fact this is obviously VB and in your first code chunk you added a C++ style comment?
     

    Yes, it is.

    :|

    Truthfully, it's probably a bit of repressed passive aggressive anger. A little piece of a real language stabbing from hell's heart at the mockery that is VB.

     @DaveK said:

    And presumably the db isn't locked between those two queries, so the
    second one could in fact return more or less rows than the dpth that was
    just counted?

    Didn't even think of that shit cherry on this mucus sundae.



  • @Cassidy said:

    Copy-pasta code?

     

    And speaking of repressed memories-- I just remembered that this code is a copy-pasta. As in, there is a SECOND query in the same code block. The one quoted above searches Staff, the second query searches Locations.

    And since no one's ever heard of UNION-- the developer copied the entire block and pasted it-- excepted all the variables are appended with "1".  dpth1, oReader1, motherfuckingshit1.

    So not only does this do a stupidly large search (poorly), it does it FOUR TIMES!

     



  • @Lorne Kates said:

    And since no one's ever heard of UNION-- the developer copied...
     

    Cue Blakeyrant about (app) developers not understanding databases in ... 3... 2... 1...



  • @joe.edwards said:

    You know, the obj doesn't bother me nearly as much as the 44.

    Completely agreed

    Are there actually 44 connection objects? And if not, what insane history made it so the object now has a 44 appended to it's name :S



  • @Aeolun said:

    @joe.edwards said:
    You know, the obj doesn't bother me nearly as much as the 44.

    Completely agreed

    Are there actually 44 connection objects? And if not, what insane history made it so the object now has a 44 appended to it's name :S

    My guess is that strSqlStatement44 is a hint that they have a list of SQL statements squirreled away. They probably heard of a message table at some point, and tried to reproduce it. Carrying the number of the query into the connection and the command are just there for context. I'm sure the original dev thought it was a great moment in maintainability.


  • SockDev

    @RTapeLoadingError said:

    counted by the database by looping through the results with a cursor
     

    Or COUNT(*)



  • My guess is that strSqlStatement44 is a hint that they have a list of SQL statements squirreled away. They probably heard of a message table at some point, and tried to reproduce it. Carrying the number of the query into the connection and the command are just there for context. I'm sure the original dev thought it was a great moment in maintainability.

    Sadly, no. I don't know the precise history of how this occurred, but they're all hand-written. Just for fun, I searched for 'strSqlStatement' and here are the results (each one also has a corresponding 'objConnection' and 'objDataReader' numbered similarly):

    'strSqlStatement' -- declared 11 separate times

    'strSqlStatement2' -- declared twice

    'strSqlStatement5' -- once

    'strSqlStatement7' -- once

    'strSqlStatement44' -- once

    Your guess is a good as mine. I didn't write this, I just have to maintain it.


Log in to reply
 

Looks like your connection to What the Daily WTF? was lost, please wait while we try to reconnect.