• BiggBru (unregistered)

    Yowza! I would post an intelligent reply to this, but I'm too busy popping my eyes out of my eye sockets.

    [:|]

  • TJ (unregistered)

    OMFG....

  • SecureD (unregistered) in reply to BiggBru

    Yeah,

    I remember from my first work experience they had some kind of equel solution :d Sad it happens to often ;)

  • ZoolEire (unregistered)

    Holy retarded programmer batman :-)

    ugh and there is 80 parameters

  • CLS (unregistered) in reply to BiggBru

    Wow.  All I can say is.... wow.

  • (cs)

    I think my favorite part is

    If Not objConn Is Nothing Then Set objConn = Nothing
    If Not objCmd Is Nothing Then Set objCmd = Nothing

    You never know - setting a variable that's already null to Nothing might blow up your server.

  • (cs)

    He could at least have written
    With objCmd.Parameters
    and saved himself (or rather, the copy&paste mechanism) SOME typing...

  • (cs)

    This code would be much better if the SQL command string was built with Javascript.

  • Steve (unregistered)

    It just gets better:

    PREV_EMP3_ADDR_1

    Meaning there is a PREV_EMP1 and PREV_EMP2 rather than putting that information into it's own table.

    I came across a similar db impl where each company had:

    DEPT_1_NAME, DEPT_1_PHONE, DEPT_1_EXT, DEPT_1_CONTACT ....  DEPT_10_NAME...

    As part of the company table.

  • (cs)

    This code is a well defined summatory of WTFs...
    [:S]

  • (cs)

    Great Merciful Crap.[:|]

    Will this even work?!? I see that he’s dynamically creating the ‘declare’ section of a PL/SQL block, and creating the parameters in the declare block, but there is no ‘begin’ or ‘end’ to the block….and WHY is he creating a PL/SQL block?!? The PL/SQL block is malformed and would not work at all….I’m guessing that when he does call the stored proc that the command object’s CommandText ignores the first part of that crap and just calls the SAVE_APP_DATA stored proc?!

     <?xml:namespace prefix = o ns = "urn:schemas-microsoft-com:office:office" /><o:p></o:p>

    Am I off on my thinking? I probably am since this code caused me to jam my fingers into my eyes and I'm having trouble seeing right now.

     

  • (cs) in reply to skicow

    Argh, my eyes!

  • (cs)

    I really feel for VB programmers. I suspect y'all have to deal with this kind of crap on a regular basis due to the (possibly erroneous, correct me if I'm wrong) notion that a lot of VB programmers got into programming via VB and couldn't tell a stack from a tree if it bit their noses off, AND you have to deal with VB's truly despicable syntax.

    Ew. Ew. Ew. Ew. Ew. Even Lisp and Prolog are less vile than this VB tripe.

    Ok, not really, but almost.

  • Hal (unregistered)

    Me new to vb.net... me click first intellisense overload saw.

  • (cs)

    [:@][:@][:@]

    And another [:@]!

    It might be excusable for a junior to use unparameterized queries (god knows I did it at first). But man, this! This makes so angry I'm gonna go beat up some homeless guy!

    Worst of all is, this guy must've known SOMETHING about how Command and the Parameters collection work. But he never got to the point of saying: "Ya know, I'm sure they wouldn't have given me a params collection just to make my life harder..."

    [:@]

  • Anonymous (unregistered)

    couple of things ..
    the ADODB reference suggests this is Upsized Code from probably VB6 ...

    he/she already has the stored proc in MSSQL
    it is likely that the RDBMS's were also switched as a part of the upgrade ..
    ie. from VB6/MSSQLServer to VB.Net/Oracle as VARCHAR2 is an oracle datatype

    this is half-ported code ... probably a consultant that was given 2 days to finish it ..

  • (cs) in reply to Anonymous

    Anonymous:
    couple of things ..
    the ADODB reference suggests this is Upsized Code from probably VB6 ...

    Upsized from VB6? I see nothing to indicate this is anything but VB6 code...

     

  • (cs)

    Ouch!! My Frik'in Eyes.  Must go to therapist now to regain my sanity. [;)]

  • PD (unregistered) in reply to rogthefrog

    Surely the best bit is


    HandleError:
      Dim eNbr$, eSrc$, eDsc$
      eNbr = Err.Number
      eSrc = Err.Source
      eDsc = Err.Description

      Err.Raise vbObjectError + eNbr, eSrc, eDsc & " " & strSQL

    it is a work of pure genius - copy all error parameters to variables that are used for nothing more than re-raising the error. Which is all it ever does - why even have an error handler; except for the fact that the author of the code heard something about vbObjectError and how you add it to error numbers so he does, even to other peoples error numbers. (Makes you wonder if he adds it to his own errors then re-throws them as well...)
  • smitty_one_eachj (unregistered)

    I really want to find some configuration circumstance where the coder wasn't given permissions to do the Right Thing, and did this instead...

  • drew (unregistered) in reply to rogthefrog
    rogthefrog:

    I think my favorite part is

    If Not objConn Is Nothing Then Set objConn = Nothing
    If Not objCmd Is Nothing Then Set objCmd = Nothing

    You never know - setting a variable that's already null to Nothing might blow up your server.

    ...if memory serves, in vb6, setting a 'nothing' variable to Nothing does indeed cause an error..

  • (cs) in reply to drew
    Anonymous:
    rogthefrog:

    I think my favorite part is

    If Not objConn Is Nothing Then Set objConn = Nothing
    If Not objCmd Is Nothing Then Set objCmd = Nothing

    You never know - setting a variable that's already null to Nothing might blow up your server.

    ...if memory serves, in vb6, setting a 'nothing' variable to Nothing does indeed cause an error..

    nope...this

     Dim obj As Object
      
      Set obj = CreateObject("Excel.Application")
       
      Set obj = Nothing
      Set obj = Nothing
      Set obj = Nothing
      Set obj = Nothing
      Set obj = Nothing
      Set obj = Nothing
      Set obj = Nothing
      Set obj = Nothing
      Set obj = Nothing
      Set obj = Nothing
      Set obj = Nothing

     

    gives no errors in VB6...

  • mkbosmans (unregistered) in reply to redtetrahedron

    Of course, al this would not have happened if the author had used XML to send the parameters to the DB. Come on, we live in the 21st century now!

  • Division Byzero (unregistered) in reply to redtetrahedron

    Nothing beats superfluous nothingness.

  • (cs) in reply to Division Byzero

    Anonymous:
    Nothing beats superfluous nothingness.

    So I got a little carried away with copy/paste... [:P]

  • Andy (unregistered) in reply to rogthefrog
    rogthefrog:

    I think my favorite part is

    If Not objConn Is Nothing Then Set objConn = Nothing
    If Not objCmd Is Nothing Then Set objCmd = Nothing

    You never know - setting a variable that's already null to Nothing might blow up your server.

    I think you have this logic backwards, he's checking for not null and setting to null. The Not obj Is Nothing is part of the syntax of VB. VB doesn't have a obj != null comparison, all you can do is say !(obj == null)

    To translate this to a curly-bracket/semicolon language you'd get:

    if(objConn != null) objConn = null
    if(objCmd != null) objCmd = null

    Still, as a whole this code is pure WTF, but for this example, it's the syntax of VB that's causing confusion....

    -Andy

  • Barry (unregistered)

    Oh My!

    I'm sitting here with my mouth hanging open.....

    I can't believe that someone managed to figure out how to do that. It looks 10 times more complicated !!!

    Hey maybe they heard that saying that you should make your code so hard t oread that no one else can change it!

  • Andy (unregistered) in reply to Andy
    Anonymous:
    rogthefrog:

    I think my favorite part is

    If Not objConn Is Nothing Then Set objConn = Nothing
    If Not objCmd Is Nothing Then Set objCmd = Nothing

    You never know - setting a variable that's already null to Nothing might blow up your server.

    I think you have this logic backwards, he's checking for not null and setting to null. The Not obj Is Nothing is part of the syntax of VB. VB doesn't have a obj != null comparison, all you can do is say !(obj == null)

    To translate this to a curly-bracket/semicolon language you'd get:

    if(objConn != null) objConn = null
    if(objCmd != null) objCmd = null

    Still, as a whole this code is pure WTF, but for this example, it's the syntax of VB that's causing confusion....

    -Andy

    D'oh, hand smacks forhead. I see what' you're getting at now. Why check for not null at all, and just set to null..... You know I see this kind of thing a lot, it all languages. Not suprised that award-winning code like this does it.... yeah this should just be objConn = null; and leave it....

    -Andy

  • Brill (unregistered)

    Yikes... could be a case of two different developers though, one that knew what they were doing, and one that didn't... however the one that didn't might have taken a moment to look up what the previous developer was doing.

  • Mike Ripper (unregistered) in reply to rogthefrog

    Thank you for your condolences. A VB6 maintenence programming job is a very sad one indeed, especially when you care about coding. However, its better than being an old school Spaghetti BASIC maintenence programmer.

  • (cs) in reply to Andy

    No, you missed the point altogether. There's no need to test whether objConn and objCmd are null/Nothing/whatever if you're going to set them to null/Nothing.

    Pseudo code

    if objConn is not null, make it null

    else (i.e., if objConn is already null), do nothing cuz, y'know, it's already null

    You want objConn to be nulled, so setting it to null/Nothing doesn't require testing whether it's not null, since nulling a null object doesn't change its state.

  • (cs) in reply to Andy

    I posted my reply just as you were posting yours, so [H]

  • No one cares (unregistered) in reply to rogthefrog

    rogthefrog:

    No, you missed the point altogether. There's no need to test whether objConn and objCmd are null/Nothing/whatever if you're going to set them to null/Nothing.

    ....

     

    I posted my reply just as you were posting yours, so [H]

     

    Really??

    Check the time...

    Andy Today, 7:40 PM

    rogthefrog Today, 8:49 PM

    Next time, read all the thread before posting redundant facts, asshole.

  • (cs) in reply to No one cares

    I'd really really really love to see the stored procedures this guy makes!

  • arse (unregistered)

    WTF is a 'Quire'???? WTF!!!!

  • Max Kington (unregistered)

    Not just for the sake of being contrary but this is a perfectly valid approach.

    Parameterizing queries does more than just allow the database to escape stuff for you.  Way more.  In Oracle there are various levels of parsing, broadly split into:

    1) A hard parse, I've never seen this SQL before

    2) A soft parse, I've seen this SQL before but you're just giving me updated bind variables.

    With generated sql you have to hard parse everytime you change the values in your concatinated string because the database views them as seperate queries. Hard parses are very expensive, especially when you're running a few million queries, and because you have a limited amount of space in the query cache, correctly and more effeciently bound SQL gets ejected because of the clutter, causing nicely bound SQL to have to be hard parsed again.  Hard parse rate on a running OLTP system should be next to 0.

    What you can do with a combination of the both is generate some standard varients of your sql, say you bind in some values that do change and have some standard ORDER BY additions you add onto the end.  that way you might end up with three correctly bound pieces

    I agree the code is nasty but from an ethos perspective this is ok. Although, per database, YMMV.

  • vhawk (unregistered)

    :|  Outch !!!   What I enjoy most of all is the amount of extra work and complexity inexperienced programmers can cause themselves

  • Jan (unregistered)
    Alex Papadimoulis:

    In the former, you just concatenate a value to your query string (<font face="Courier New" size="2">"where col='" + val + "'"</font>)



    Of course, you'd use & not + to do that....


  • (cs) in reply to Andy
    Anonymous:
    Anonymous:
    rogthefrog:

    I think my favorite part is

    If Not objConn Is Nothing Then Set objConn = Nothing
    If Not objCmd Is Nothing Then Set objCmd = Nothing

    You never know - setting a variable that's already null to Nothing might blow up your server.

    I think you have this logic backwards, he's checking for not null and setting to null. The Not obj Is Nothing is part of the syntax of VB. VB doesn't have a obj != null comparison, all you can do is say !(obj == null)

    To translate this to a curly-bracket/semicolon language you'd get:

    if(objConn != null) objConn = null
    if(objCmd != null) objCmd = null

    Still, as a whole this code is pure WTF, but for this example, it's the syntax of VB that's causing confusion....

    -Andy

    D'oh, hand smacks forhead. I see what' you're getting at now. Why check for not null at all, and just set to null..... You know I see this kind of thing a lot, it all languages. Not suprised that award-winning code like this does it.... yeah this should just be objConn = null; and leave it....

    -Andy

    The real reason to check for not null (Not var Is Nothing) in VB6 is to close the object before setting to null, or you'll just end up having objects floating around with no more references to it, taking up memory space (this is most annoying in ASP as the variables will stay there as long as the IIS application is running, therefore ending up jamming the server.

    But here the guy just sets them to Null, that's a WTF.  There is what it should be, of course making sure the objects are open before as well or doing "Resume Next" so not to crash if it is already closed:

    If<FONT color=#000000> </FONT>Not<FONT color=#000000> objCmd </FONT>Is<FONT color=#000000> </FONT>Nothing<FONT color=#000000> </FONT>Then
       <FONT color=#000000>objCmd.Close
    </FONT>       Set<FONT color=#000000> objCmd = </FONT>Nothing
    End If
    If     Not objConn Is Nothing Then 
       <FONT color=#000000>objConn.Close</FONT>
       Set     objConn = Nothing
    End If

  • (cs) in reply to No one cares
    Anonymous:

    rogthefrog:

    No, you missed the point altogether. There's no need to test whether objConn and objCmd are null/Nothing/whatever if you're going to set them to null/Nothing.

    ....

     

    I posted my reply just as you were posting yours, so [H]

     

    Really??

    Check the time...

    Andy Today, 7:40 PM

    rogthefrog Today, 8:49 PM

    Next time, read all the thread before posting redundant facts, asshole.

    Erm... why would I waste my time clarifying something someone misunderstood AFTER they post that they didn't actually misunderstand it? Ever thought someone's time zone might be screwed up?

  • (cs) in reply to rogthefrog

    A great big WTF to everyone even talking about the best technique to use when setting object variables to Nothing in VB. 

    You don't need to do this unless you are using static variables; as soon as the variable is out of scope the reference count on the object is decremented the same as if you set an object variable equal to Nothing.  

    Setting an object variable to Nothing isn't like you are explicitly freeing memory or anything.

  • uber (unregistered) in reply to Jeff S

    This goes beyond WTF and into the realm of blasphemy.

  • IndyHarcourt (unregistered) in reply to TJ

    That was my first thought as well. 
    Damn!

  • (cs) in reply to IndyHarcourt

    Alex, I completely forgot to mention, I noticed yesterday you forgot to attribute this wtf.

    Setting an object to null or nothing will run its finalizer on the next garbage collection in vb or .net. VB6 garbage collection is at the end of the sub/function, ASP is usually at the end of the page unless memory fills up, ASP.net interrupts to run every second or so. iirc, it's been a while. External custom resources in ASP are usually not implicitly freed, a bug in IIS5, so it's always a good idea to "destroy" them, but of course it's incredibly silly to set something only if it isn't already. (Maybe he heard somewhere that variable writes were much more expensive than reads?)

  • MWK (unregistered) in reply to Jeff S
    Jeff S:

    A great big WTF to everyone even talking about the best technique to use when setting object variables to Nothing in VB. 

    You don't need to do this unless you are using static variables; as soon as the variable is out of scope the reference count on the object is decremented the same as if you set an object variable equal to Nothing.  

    Setting an object variable to Nothing isn't like you are explicitly freeing memory or anything.

    Actually, setting a reference to Nothing VB does indeed free up memory since we are talking about COM pointers behind the scenes. By explicitly setting a variable to Nothing, the reference count of the COM object gets decremented (IUnknown::Release gets called). Once the reference count goes to zero, the object is destroyed.

    Of course, you could also let the variable fall out of scope in which case it its reference count is also decremented. It's been awhile but I think there was a fluke in VB some versions back where it was necessary to set a variable to Nothing to ensure that the reference count was decremented (thus ensuring no resource leaks).

  • (cs)

    Okay, I see nothing wrong here, it looks like good code, I might even implement something similar in a project I currently working on.

    Oh yes, and I recently built my house out of porrige and cherrio's.

    <FONT size=5>WTF!</FONT>

  • (cs) in reply to foxyshadis

    foxyshadis:
    Alex, I completely forgot to mention, I noticed yesterday you forgot to attribute this wtf.

    The source was anonymous ...

    ... well, actually, it was me, but I wanted to be anonymous (on the main post, at least) since it was code I uncovered pretty recently here at work. Obfuscated the variable/tables and even the CWs didn't pick up on its origin ;-)

  • (cs) in reply to MWK
    Anonymous:
    Jeff S:

    A great big WTF to everyone even talking about the best technique to use when setting object variables to Nothing in VB. 

    You don't need to do this unless you are using static variables; as soon as the variable is out of scope the reference count on the object is decremented the same as if you set an object variable equal to Nothing.  

    Setting an object variable to Nothing isn't like you are explicitly freeing memory or anything.

    Actually, setting a reference to Nothing VB does indeed free up memory since we are talking about COM pointers behind the scenes. By explicitly setting a variable to Nothing, the reference count of the COM object gets decremented (IUnknown::Release gets called). Once the reference count goes to zero, the object is destroyed.

    Of course, you could also let the variable fall out of scope in which case it its reference count is also decremented. It's been awhile but I think there was a fluke in VB some versions back where it was necessary to set a variable to Nothing to ensure that the reference count was decremented (thus ensuring no resource leaks).

    [8-)] Not quite sure how what you wrote is different from what I wrote ...

  • em (unregistered) in reply to Jeff S
    Jeff S:

    A great big WTF to everyone even talking about the best technique to use when setting object variables to Nothing in VB. 

    You don't need to do this unless you are using static variables; as soon as the variable is out of scope the reference count on the object is decremented the same as if you set an object variable equal to Nothing.  

    Setting an object variable to Nothing isn't like you are explicitly freeing memory or anything.

    My impression is that TONS of programmers just don't understand this concept at all. Check some of the comments in this Slashdot thread. There's a good contingent of people who have the firmly entrenched belief that you need to set variables to null in Java when you're done with them, so the garbage collector can reclaim them...

  • (cs)

    There is still one catch nothing the function line

    Public Function SaveApplication(ByVal varrApplicationData As Variant) As Variant

    <FONT color=#000000>You expect to return a variant. Actually it returns true or nothing at all. Well true and nothing can fit in a variant.</FONT>

    And

    <FONT face="Courier New">   <FONT size=2>If</FONT><FONT size=2><FONT color=#000000> objParam.Type = adVarChar </FONT>Then
    <FONT color=#000000>      strSQL = strSQL & </FONT>"in"<FONT color=#000000> & objParam.Name & </FONT>" VARCHAR2("<FONT color=#000000> & objParam.Size & </FONT>");"</FONT></FONT><FONT face="Courier New"><FONT size=2><FONT color=#000000> & vbCrLf
        </FONT>ElseIf<FONT color=#000000> objParam.Type = adChar </FONT>Then
    <FONT color=#000000>      strSQL = strSQL & </FONT>"in"<FONT color=#000000> & objParam.Name & </FONT>" CHAR("<FONT color=#000000> & objParam.Size & </FONT>");"</FONT></FONT><FONT face="Courier New"><FONT size=2><FONT color=#000000> & vbCrLf
        </FONT>Else
    <FONT color=#000000>      strSQL = strSQL & </FONT>"in"<FONT color=#000000> & objParam.Name & </FONT>" VARCHAR2("<FONT color=#000000> & objParam.Size & </FONT>");"</FONT></FONT><FONT face="Courier New"><FONT size=2><FONT color=#000000> & vbCrLf
        </FONT>End<FONT color=#000000> </FONT>If</FONT></FONT>

    can be written like

    <FONT face="Courier New"><FONT size=2>   If<FONT color=#000000> objParam.Type = adChar </FONT>Then
    <FONT color=#000000>      strSQL = strSQL & </FONT>"in"<FONT color=#000000> & objParam.Name & </FONT>" CHAR("<FONT color=#000000> & objParam.Size & </FONT>");"</FONT></FONT><FONT face="Courier New"><FONT size=2><FONT color=#000000> & vbCrLf
       </FONT>Else
    <FONT color=#000000>      strSQL = strSQL & </FONT>"in"<FONT color=#000000> & objParam.Name & </FONT>" VARCHAR2("<FONT color=#000000> & objParam.Size & </FONT>");"</FONT></FONT><FONT face="Courier New"><FONT size=2><FONT color=#000000> & vbCrLf
       </FONT>End<FONT color=#000000> </FONT>If
    </FONT></FONT>

Leave a comment on “The Best of Both Worlds”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article