Comment On A Touch of Genius

FIRST

FTFY:

If bGroup1 = 0 Then

  bGroup = False

ElseIf bGroup1 = 1 Then

  bGroup = True

ElseIf bGroup1 = 2 Then

  bGroup = FileNotFound

EndIf

"More JavaScript. Is it a string or an integer?"
if (obj.value>="9000" && obj.value<=9999)

Vegeta, what does the scouter say about his level?
IT'S OVER NINE THOUSAND!!!!!

I'm not really familiar with VB.net, so can anyone explain what's wrong with "How to execute a „complicated“ SQL in a stored procedure? Piece of cake!"?

BadFellas.org:

I'm not really familiar with VB.net, so can anyone explain what's wrong with "How to execute a „complicated“ SQL in a stored procedure? Piece of cake!"?

That is not VB .NET; it's an SQL stored procedure. An SQL stored procedure that builds what is essentially a static SQL query as a string (VARCHAR) and then executes the contents of that string.

I'm guessing the snipped lines looked something like:

sSql = Replace(sSql, "WHERE  And", " WHERE ")

sSql = Replace(sSql, "WHERE  AND  ", " WHERE ")

and so on. So many possibilities! I love being paid by lines of code!

BadFellas.org:

I'm not really familiar with VB.net, so can anyone explain what's wrong with "How to execute a „complicated“ SQL in a stored procedure? Piece of cake!"?

Its a WTF in any language, because it goes the opposite direction, taking parameters and building a single string to execute. That pattern is how SQL Injection sneaks in. Instead, you're supposed to pass the parameters to a predefined query with a designated slot for each parameter.

Some of that code slightly makes sense. I have used old VB and ended up writing some pretty stupid looking code to get around some very stupid language "features".

A perfect example was when a single x+=1 had to be replaced with x=x+1. Why the first wouldn't work where it was while the second worked fine made no sense, but it made the code function. I can see someone following me and thinking that I was a dolt.

If bGroup1 = 0 Then

  bGroup = False

ElseIf bGroup1 = 1 Then

  bGroup = True

ElseIf bGroup1 = 2 Then

  bGroup = True

EndIf

Clearly my brain is not yet sufficiently caffeinated this morning (or it's my lack of VB?). What's the major fault here?

captcha: paratus (latin, "ready"). I must not be ready to read WTFs yet today?

They just made it so changes are super easy. I get confused when people start using OR and I'm sure everybody else does as well.

OhDear:

A perfect example was when a single x+=1 had to be replaced with x=x+1. Why the first wouldn't work where it was while the second worked fine made no sense, but it made the code function. I can see someone following me and thinking that I was a dolt.

the += assignment operator isn't (and never has been) a feature in basic or visual basic.
VB's not a bad language but this is also a little quibble of mine.

erm...

bGroup = (bGroup1 != 0)

fw:

erm...

bGroup = (bGroup1 != 0)

Yes, you'd think so, but you actually dropped a code path: the implied

ElseIf bGroup1 = 3 Then

  bGroup = 'undefined'

Kozz:

If bGroup1 = 0 Then

  bGroup = False

ElseIf bGroup1 = 1 Then

  bGroup = True

ElseIf bGroup1 = 2 Then

  bGroup = True

EndIf

Clearly my brain is not yet sufficiently caffeinated this morning (or it's my lack of VB?). What's the major fault here?

captcha: paratus (latin, "ready"). I must not be ready to read WTFs yet today?

I guess this would be to easy?
bGroup = (bGroup1 != 0);

OhDear:

Some of that code slightly makes sense. I have used old VB and ended up writing some pretty stupid looking code to get around some very stupid language "features".

A perfect example was when a single x+=1 had to be replaced with x=x+1. Why the first wouldn't work where it was while the second worked fine made no sense, but it made the code function. I can see someone following me and thinking that I was a dolt.

Oh dear, maybe it's because there was no such operator in plain old VB? (I don't know about VB.Net, as I managed to avoid it)

"DECLARE @QUERY AS VARCHAR(1000)"

First time I've WTFed out loud in a while.

If sCompanyGroup = "" Then

  Field46 = ""

Else

  Field46 = sCompanyGroup

End If

This one might not be as stupid as it seems. In VB, the comparison somestring = "" evaluates true if the string is empty or if it's Nothing (null for non-VB coders). Comparing to an empty string in VB is equivalent to calling String.IsNullOrEmpty().

Therefore the above code prevents assigning Nothing (null) to the field.

> If bGroup = True Then

Just that line is enough to make me want to slap the developer with a wet haddock. To be really sure bGroup (presumably a Boolean) is true, why not
If (bGroup = True) = True Then
or
If ((bGroup = True) = True) = True Then
or (continue ad nauseam)

ElseIf bGroup1 = 1 Then

  bGroup = True

ElseIf bGroup1 = 2 Then

  bGroup = True

Actually he's on the right track. I require my team to separately enumerate each possible value, even if there are 50 or more.

1. You can generate the lines of code with a simple script.
2. It forces you to think about -- and document -- every possibility, and clearly shows the next coder which cases you aren't prepared to handle.
3. If the requirements suddenly change for, say, bGroup1 = 37, you can go right to that case and update it without damaging the structure of the surrounding code.
4. The compiler will optimize it all anyway. Better than you can.

I guess this would be to easy?
bGroup = (bGroup1 != 0);

I'd considered that, but my lack of VB made me wonder whether bGroup was your standard primitive boolean (I inherently distrust coders using Hungarian notation), how strictly typed is VB, and therefore whether bGroup could possibly take on values other than True or False (from your comment, I assume it cannot).

Maybe it's best I stick with the languages I know. ;)

SQuirreL:

Yes, you'd think so, but you actually dropped a code path: the implied

ElseIf bGroup1 = 3 Then

  bGroup = 'undefined'

I think you mean

ElseIf bGroup1 > 2 Then

  bGroup = 'FileNotFound'

If bGroup1 = 0 Then
bGroup = False
ElseIf bGroup1 = 1 Then
bGroup = True
ElseIf bGroup1 = 2 Then
bGroup = True
EndIf

I see nothing wrong with this. Easy to read, and easy to maintain. Just because it could be rewritten (depending on whether bGroup = NULL is a valid code path) to bGroup = bGroup1 != 0 doesn't mean it's the right way to go, and only a college student who has never worked in a professional environment would write such pre-mature optimization.

What happens when bGroup1 is extended? Continue to add on your boolean expression? Or perhaps then you feel you would rewrite it with IF statements? I hope you have unit tests.

"bGroup = (bGroup1 != 0);"

This line is C, not? Isn't in VB "<>", not "!="?
Then, maybe (not VB):

bGroup = (bGroup1<3) ? (bGroup1 != 0) : bGroup;

Sven:

This one might not be as stupid as it seems. In VB, the comparison somestring = "" evaluates true if the string is empty or if it's Nothing (null for non-VB coders). Comparing to an empty string in VB is equivalent to calling String.IsNullOrEmpty().

Therefore the above code prevents assigning Nothing (null) to the field.

If an 'as designed' feature of a programming language makes it to this site then one might wonder where the real problem lies. Is it the programmer or is it ... ?

Sorry, but bGroup1 is one horribly named variable. It reeks of laziness, as its Hungarianish name doesn't even describe its type, let alone its function.

Erick:

Sorry, but bGroup1 is one horribly named variable. It reeks of laziness, as its Hungarianish name doesn't even describe its type, let alone its function.

It is a command variable. "be Group one". Can't you see that?

Excellent suggestions, TopCod3r. I can always count on your sage advice. *wink*

if (obj.value>="9000" && obj.value<=9999)

It's OVER 9000!!!!!!

HAH! beat you at #227641

Virtually Brillant:

> If bGroup = True Then

Just that line is enough to make me want to slap the developer with a wet haddock. To be really sure bGroup (presumably a Boolean) is true, why not
If (bGroup = True) = True Then
or
If ((bGroup = True) = True) = True Then
or (continue ad nauseam)

In VB, the equality comparison operator is the same as the assignment operator. In this instance it is a valid comparison, though it would have been better to use "If bGroup Then".

Matthias:

If bGroup1 = 0 Then
bGroup = False
ElseIf bGroup1 = 1 Then
bGroup = True
ElseIf bGroup1 = 2 Then
bGroup = True
EndIf

I see nothing wrong with this. Easy to read, and easy to maintain. Just because it could be rewritten (depending on whether bGroup = NULL is a valid code path) to bGroup = bGroup1 != 0 doesn't mean it's the right way to go, and only a college student who has never worked in a professional environment would write such pre-mature optimization.

What happens when bGroup1 is extended? Continue to add on your boolean expression? Or perhaps then you feel you would rewrite it with IF statements? I hope you have unit tests.

No, if you really wanted to do that, then you should use Select/Case statements:

Select Case bGroup1

   case 0:

      bGroup1 = False

   case 1, 2:

      bGroup = True

   ' extend here

End Select

Much easier to read and extend.

TopCoder: "3. If the requirements suddenly change for, say, bGroup1 = 37, you can go right to that case and update it without damaging the structure of the surrounding code. "

So it is hard in your team to change something like

<boohoo surrounding code wtf>
if (1 == bGroup1 && 2 == bGroup1) {
}
<zomg more surrounding code>

to

<boohoo surrounding code wtf>
if (1 == bGroup1) {
} else if (2 == bGroup1) {
}
<zomg more surrounding code>

?

In professional engineering there is also the paradigm to do work only when it has to be done.

It is another thing if you have a switch on some enum, because the compiler will be able to warn you if you forgot an enum-case (which saved my arse several times when I was writing parsers), but that's another story.

Sven:

If sCompanyGroup = "" Then

  Field46 = ""

Else

  Field46 = sCompanyGroup

End If

This one might not be as stupid as it seems. In VB, the comparison somestring = "" evaluates true if the string is empty or if it's Nothing (null for non-VB coders). Comparing to an empty string in VB is equivalent to calling String.IsNullOrEmpty().

Therefore the above code prevents assigning Nothing (null) to the field.

Um, maybe I'm wrong here, but null is not the same as an empty string (except in Oracle).

A VB statement that tests if the value = "" will return false if the value is instead NULL. It's common to force the null to a string value using the NZ function (or to 0 if the field is numeric) and then test it, thus covering both possibilities.

If nz(sCompanyGroup, "") = "" then ...

Amerrickangirl:

Sven:

In VB, the comparison somestring = "" evaluates true if the string is empty or if it's Nothing (null for non-VB coders).

A VB statement that tests if the value = "" will return false if the value is instead NULL.

As horrifying as it sounds, I just verified this with VB 2008. I ran the statement

If "" = Nothing Then Throw New Exception("VB sucks")

It threw the exception.
Then I ran

If "" is Nothing Then Throw New Exception("VB sucks")

It didn't throw the exception.
So it looks like the = operator for comparing strings is horribly broken in VB.

If bGroup1 = 0 Then
bGroup = False
ElseIf bGroup1 = 1 Then
bGroup = True
ElseIf bGroup1 = 2 Then
bGroup = True
EndIf

this isn't a WTF, at least not as presented. if bgroup1 is 0 you want to set bgroup to false, if it is 1 or 2, you want to set bgroup to true, if it is anything else, you want to leave bgroup as whatever it already was.

You could say that it would have been better to right

ElseIf bGroup1 = 1 OR bGroup1 = 2 Then

bGroup = True

But that would have been less efficient since VB doesn't short circuit. If this was written after the introduction of OrElse, then it is a very minor wtf, at most.

The bigger wtf would be the descriptions of the WTF's.

If the variable contains an empty string

to describe this

If bResult = "" Then

is seriously fucked up.

OutWithTheTroll:

Kozz:

If bGroup1 = 0 Then

  bGroup = False

ElseIf bGroup1 = 1 Then

  bGroup = True

ElseIf bGroup1 = 2 Then

  bGroup = True

EndIf

Clearly my brain is not yet sufficiently caffeinated this morning (or it's my lack of VB?). What's the major fault here?

captcha: paratus (latin, "ready"). I must not be ready to read WTFs yet today?

I guess this would be to easy?
bGroup = (bGroup1 != 0);

yes, that would be too easy. it would also be wrong.

The way I read the code, without further details or any of the preceding code is that if the value is 0, set bgroup to false, if it is 1 or 2, set it to true, if it is anything else, leave it alone. Your method kinda screws up that last part.

No, vb does not evaluates TRUE if comparing "" with null/nothing. It raises an error.

Thats why you can see this gems sometimes:

if sCompanyGroup & "" = "" then

 ...

End if

Just to not have to check for null/nothing.
(If you add "" to nothing you get "") ;)

SQuirreL:

BadFellas.org:

I'm not really familiar with VB.net, so can anyone explain what's wrong with "How to execute a „complicated“ SQL in a stored procedure? Piece of cake!"?

Its a WTF in any language, because it goes the opposite direction, taking parameters and building a single string to execute. That pattern is how SQL Injection sneaks in. Instead, you're supposed to pass the parameters to a predefined query with a designated slot for each parameter.

Not at all! The SP in question is getting type checked parameters as input. Observe all the "convert" statements.
Thus, no chance for injection.

It can be very meaningful to create SQL on the fly like this. An example could be a very large set of
"and (field=@value or @value is null) kinds of variable-parameter constructs.

OutWithTheTroll:

Kozz:

If bGroup1 = 0 Then

  bGroup = False

ElseIf bGroup1 = 1 Then

  bGroup = True

ElseIf bGroup1 = 2 Then

  bGroup = True

EndIf

Clearly my brain is not yet sufficiently caffeinated this morning (or it's my lack of VB?). What's the major fault here?

captcha: paratus (latin, "ready"). I must not be ready to read WTFs yet today?

I guess this would be to easy?
bGroup = (bGroup1 != 0);

Rephrasing as
bGroup = !!bGroup1;
still doesn't undefine bGroup when bGroup1 is bigger than two.

Welbog:

Amerrickangirl:

Sven:

In VB, the comparison somestring = "" evaluates true if the string is empty or if it's Nothing (null for non-VB coders).

A VB statement that tests if the value = "" will return false if the value is instead NULL.

As horrifying as it sounds, I just verified this with VB 2008. I ran the statement

If "" = Nothing Then Throw New Exception("VB sucks")

It threw the exception.
Then I ran

If "" is Nothing Then Throw New Exception("VB sucks")

It didn't throw the exception.
So it looks like the = operator for comparing strings is horribly broken in VB.

Null and nothing are NOT the same.

http://www.evolt.org/article/Empty_Nothing_and_Null_How_do_you_feel_today/17/346/index.html

"In VB there are three states of a variable not having a value (well, a usable one anyway).

Empty == Uninitialized Null == actually, well, Null. Okay, so technically this is a value. More in a sec.

Nothing == for objects only, basically the same as Empty

"" --the empty string (VBScript calls it the null string) is a valid value. Remember this.

VBScript also has three ways to check for non values: IsEmpty, IsNull, and IsNothing. The lesson is that you can't mix and match. And none of them will cooperate if you're checking against "". You check for "" by comparing against ""."

sebb - hook line and sinker

sebbb:

"bGroup = (bGroup1 != 0);"

This line is C, not? Isn't in VB "<>", not "!="?
Then, maybe (not VB):

bGroup = (bGroup1<3) ? (bGroup1 != 0) : bGroup;

you are making the assumption that bGroup1 can't be negative

Amerrickangirl:

http://www.evolt.org/article/Empty_Nothing_and_Null_How_do_you_feel_today/17/346/index.html

This article was posted nine years ago for VBScript. I'm talking about a different, modern language: VB 2008.

sCodEmployee.sCodPiece.Size++

These WTFs are a sure sign of a Classic VB (or maybe even *shudder* VBScript) developer. Most of these WTFs are exactly how you needed to do things in the bad old days. VBScript especially has only rudimentary classes, and hardly anyone used them for anything.

Plus the whole:

If field = "" then
someVal = ""
else
someVal = field
end if

spiel is par for the course in Old VB.

Bob:

"DECLARE @QUERY AS VARCHAR(1000)"

First time I've WTFed out loud in a while.

I am being serious here. What is wrong with that line of SQL code?

I have seen plenty of huge SQL projects that build dynamic queries inside stored procedures exactly as shown here.

That would follow the COBOL way of things. You can do anything with enough manpower.

Actually += is part of vb.net. I literally just tested it.

sebbb:

TopCoder: "3. If the requirements suddenly change for, say, bGroup1 = 37, you can go right to that case and update it without damaging the structure of the surrounding code. "

So it is hard in your team to change something like

<boohoo surrounding code wtf>
if (1 == bGroup1 && 2 == bGroup1) {
}
<zomg more surrounding code>

to

<boohoo surrounding code wtf>
if (1 == bGroup1) {
} else if (2 == bGroup1) {
}
<zomg more surrounding code>

?

In professional engineering there is also the paradigm to do work only when it has to be done.

It is another thing if you have a switch on some enum, because the compiler will be able to warn you if you forgot an enum-case (which saved my arse several times when I was writing parsers), but that's another story.

How do people fall for this every day?

Tom:

Virtually Brillant:

> If bGroup = True Then

Just that line is enough to make me want to slap the developer with a wet haddock. To be really sure bGroup (presumably a Boolean) is true, why not
If (bGroup = True) = True Then
or
If ((bGroup = True) = True) = True Then
or (continue ad nauseam)

In VB, the equality comparison operator is the same as the assignment operator. In this instance it is a valid comparison, though it would have been better to use "If bGroup Then".

Um, that was kind of the point I was making.

Captcha: letatio *snigger*