Comment On The Best of Both Worlds

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

[:|]

OMFG....

Yeah,



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

Holy retarded programmer batman :-)

ugh and there is 80 parameters

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

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.

He could at least have written

With objCmd.Parameters

and saved himself (or rather, the copy&paste mechanism) SOME typing...

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

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.

This code is a well defined summatory of WTFs...

[:S]

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?!

 

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.

 

Argh, my eyes!

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.

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

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

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

[:@]

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

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

 

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

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

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

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

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

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!

Nothing beats superfluous nothingness.

Anonymous:

Nothing beats superfluous nothingness.

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

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

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!

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

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.

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.

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.

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

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.

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

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

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.

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

Alex Papadimoulis:

In the former, you just concatenate a value to your query string ("where col='" + val + "'")

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

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 Not objCmd Is Nothing Then
   objCmd.Close
   Set objCmd = Nothing
End If
If Not objConn Is Nothing Then 
   objConn.Close
   Set objConn = Nothing
End If

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?

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.

This goes beyond WTF and into the realm of blasphemy.

That was my first thought as well. 

Damn!

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?)

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

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.

WTF!

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 ;-)

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

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

There is still one catch nothing the function line

Public Function SaveApplication(ByVal varrApplicationData As Variant) As Variant

You expect to return a variant. Actually it returns true or nothing at all. Well true and nothing can fit in a variant.

And

   If objParam.Type = adVarChar Then
      strSQL = strSQL & "in" & objParam.Name & " VARCHAR2(" & objParam.Size & ");" & vbCrLf
    ElseIf objParam.Type = adChar Then
      strSQL = strSQL & "in" & objParam.Name & " CHAR(" & objParam.Size & ");" & vbCrLf
    Else
      strSQL = strSQL & "in" & objParam.Name & " VARCHAR2(" & objParam.Size & ");" & vbCrLf
    End If

can be written like

   If objParam.Type = adChar Then
      strSQL = strSQL & "in" & objParam.Name & " CHAR(" & objParam.Size & ");" & vbCrLf
   Else
      strSQL = strSQL & "in" & objParam.Name & " VARCHAR2(" & objParam.Size & ");" & vbCrLf
   End If