Keep on Searching


  • Trolleybus Mechanic

     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."


  • Considered Harmful

    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]


  • Trolleybus Mechanic

    @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.


  • Trolleybus Mechanic

    @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


  • ♿ (Parody)

    @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.


  • FoxDev

    @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.


  • Trolleybus Mechanic

    @mikeTheLiar said:

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

    Your company did (or still does) outsource code to the East. Because the system is large enough, chunks of the code get outsourced to different locations, sometimes multiple locations at the same time.

    At one point, the same chunk of code was being passed between an outsourcing company in India and an outsourcing company in China. In fact, they were rival companies. Bitter rivals. Not only did they hate each other on a business level, but they truly despised each other on a cultural level. You haven't seen such outright hostile racism until you've witnessed the unadulterated hate India and China can have for each other.

    The Indian developers knew that once they turned in the code, it would be passed back to the China developers, who would be working on that code-- but a separate block of it. The Chinese would have to work with the Indian's code, but wouldn't be allowed to change it. The Indians couldn't exactly sabotage the code or give the Chinese an unworkable mess-- they still had to turn in a "quality" product, lest they get fired.

    But they could fuck around with the variable names. They took the Chinese strSqlStatement8, and changed it to strSqlStatement4. And then as an extra little twist of the knife, they added a SECOND 4 to that fucker.

    They checked in that code, knowing each and every minute of each and every day, the Chinese would be forced to work under the auspicious aura of that horrific number, and there was nothing they could do about it.



  • That's ok; the Chinese renamed it "rawDrippingFattyBeef", so they got their revenge.



  • @RaceProUK said:

    @RTapeLoadingError said:

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

    Or COUNT(*)

     

    *woosh*

    @blakeyrat said:

    the Chinese renamed it "rawDrippingFattyBeef"

    Now I hunger. I hope you're pleased with yourself.

     

     



  • @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?
    pI'vm anot avery jkeen oon jHungarian nNotation.



  • @Lorne Kates said:

    But they could fuck around with the variable names. They took the Chinese strSqlStatement8, and changed it to strSqlStatement4. And then as an extra little twist of the knife, they added a SECOND 4 to that fucker.

    They checked in that code, knowing each and every minute of each and every day, the Chinese would be forced to work under the auspicious aura of that horrific number, and there was nothing they could do about it.

    Ha ha.

    To be "that guy", in Asian languages, the word for "4" sounds a lot like the word for death, and 8 sounds like wealth. They're very serious about that shit.


  • Trolleybus Mechanic

    @Nexzus said:

    To be "that guy", in Asian languages, the word for "4" sounds a lot like the word for death, and 8 sounds like wealth. They're very serious about that shit.
     

     



  • Yay, base 9!



  • @Lorne Kates said:

    Your company did (or still does) outsource code to the East. Because the system is large enough, chunks of the code get outsourced to different locations, sometimes multiple locations at the same time.

    At one point, the same chunk of code was being passed between an outsourcing company in India and an outsourcing company in China. In fact, they were rival companies. Bitter rivals. Not only did they hate each other on a business level, but they truly despised each other on a cultural level. You haven't seen such outright hostile racism until you've witnessed the unadulterated hate India and China can have for each other.

    The Indian developers knew that once they turned in the code, it would be passed back to the China developers, who would be working on that code-- but a separate block of it. The Chinese would have to work with the Indian's code, but wouldn't be allowed to change it. The Indians couldn't exactly sabotage the code or give the Chinese an unworkable mess-- they still had to turn in a "quality" product, lest they get fired.

    But they could fuck around with the variable names. They took the Chinese strSqlStatement8, and changed it to strSqlStatement4. And then as an extra little twist of the knife, they added a SECOND 4 to that fucker.

    They checked in that code, knowing each and every minute of each and every day, the Chinese would be forced to work under the auspicious aura of that horrific number, and there was nothing they could do about it.

    This story is going into the code base. Every time I see something that makes me WTF, I'll just think of those poor bastards and their long-distance, slowly simmering feud.



  • @mikeTheLiar said:

    slowly simmering feud.
     

    I just think of the wonderful slowly simmering food.



  • @dhromed said:

    I just think of the wonderful slowly simmering food.

    rawDrippingFattyBeef?



  • @Lorne Kates said:

     

    Zowie!  Three more floors and it jumps from 39 to 50.  (Or from 39 to 44 and then to 50, if the business about doubling them turns bad luck to good.)

     



  • @mikeTheLiar said:

    @dhromed said:

    I just think of the wonderful slowly simmering food.

    rawDrippingFattyBeef?

     

    Chinese foo is the best.

     



  • @dhromed said:

    Chinese foo is the best.
     

    Bar none.



  • @Cassidy said:

    @dhromed said:

    Chinese foo is the best.
     

    Bar none.


    Reminds me of a place down the street from where I lived, where the Street View image was badly stitched making the sign say "CHINESE FOO TO TAKE AWAY". Unfortunately, I don't have a screenshot, just an old link, and it seems like the street was redone and the street view error is both far more obvious and doesn't have an amusing side effect any more...


Log in to reply