The Daily WTF: Curious Perversions in Information Technology

2005-09-20 Reply Admin

JohnO:
Anonymous:
This is clearly the We-use-our-production-database-for-development-and-QA-so-we-can't-actually-run-some-sql-statements durring-development-and-QA design pattern.

I thought of this myself. But do you really wait until you are in production and a user is trying to do something with your system before you make calls to the database? Do they really think the confirm flag is a substitute for testing the actual calls?

No, it's not. I suppose I should've called it an anti-pattern.

I wasn't attempting to defend that sort of code, but I've seen it more then once. There's usually also a 'debug' flag that is hardcoded into the app so that you need to recompile to move into production as well.

jeremydwill · 2005-09-20 Reply Admin

OK, the procedure is obviously not called DeleteAccount, but it was the guts of the procedure I was talking about. (Pay no attention to the man behind the curtain.)

2005-09-20 Reply Admin

Oliver Klozoff:
the code doesn't do ANYTHING if Confirm is false, including logging a message to indicate that "we were going to delete an account here, but didn't.". What sort of testing doesn't involve verifying that the results of your test match what you expected?

The sort of testing done by people who use their production database as a development/testing database. That is to say, IDIOTS. :-)

OneFactor · 2005-09-20 Reply Admin

I wonder if the confirm boolean initially gets set by some sort of user lookup. That way, the system could support read-only / non-serious users simply by setting their confirm flag to false.

Methinks that this brainchild was conceived by a developer who got promoted into management and then had the clout to make this "security feature" a corporate standard.

dreifus · 2005-09-20 Reply Admin

Anonymous:
They forgot to utilize isTrue methods from previous WTFs.

I'm sure that's because this site is misleadingly named by its author TDWTF instead of TDPnP.

MikeB · 2005-09-20 Reply Admin

Alex Papadimoulis:

Public Sub DeleteAccount(AccountNum As String, Confirm As Boolean)

If Confirm <> True Then Exit Sub

Dim cmd As New ADODB.Command
With cmd
 .ActiveConnection = getConn()
 .CommandText = "DELETE_PLAYER_ACCOUNT"
 .Parameters.Append _
 .CreateParameter("ACCOUNT_NUM", adChar, adParamInput, 8, AccountNum)
 .Parameters.Append _
 .CreateParameter("CONFIRM", adInteger, adParamInput, 4, CInt(Confirm))
 .Execute
End With

End Sub

If MsgBox("Are your intentions true?",vbYesNo) = vbYes Then
DeleteAccount(AccountNum, True)
Else
DeleteAccount(AccountNum, False)
End if

I don't know this would work for them, but it seems that if you're really serious, you should prompt the user to find out what they think.

Seriously!

2005-09-20 Reply Admin

While idiotic, I imagine the reasoning behind this went a little like this (I don't know vb, here's the C version):

#ifdef TEST_ON_LIVE_DATABASE_WITHOUT_DESTROYING_DATA
g_dryRun = 1;
#else
g_dryRun = 0;
#endif

DeleteAccount(account_id, !g_dryRun);

Mung Kee · 2005-09-20 Reply Admin

Anonymous:
Actually, I think this code is really cute. . .

perv

Hugo · 2005-09-20 Reply Admin

What an obvious error - how can anyone claim to have >>an extra layer of conformation before doing "serious" things<< if there is only one confirmation required? Sure, we all know that the serious stuff needs at least two, preferably more confirmations: "are you sure" "are you really sure" "are you really really really really really sure" "if you proceed you'll lose important data, please click cancel unless you are definitely sure" etc.

In codeL

Public Sub DeleteAccount(AccountNum As String, Confirm As Boolean<FONT color=#000000>, DoubleCheck </FONT>As<FONT color=#000000> </FONT>Boolean)
If Confirm <> True Then Exit Sub
If DoubleCheck <> True Then Exit Sub
Dim cmd As New ADODB.Command
With cmd
.ActiveConnection = getConn()
.CommandText = "DELETE_PLAYER_ACCOUNT"
.Parameters.Append _
.CreateParameter("ACCOUNT_NUM", adChar, adParamInput, 8, AccountNum)
.Parameters.Append _
.CreateParameter("CONFIRM", adInteger, adParamInput, 4, CInt(Confirm))
.Parameters.Append _
.CreateParameter("DOUBLECHECK", adInteger, adParamInput, 4, CInt(DoubleCheck))
.Execute
End With
End Sub

Mung Kee · 2005-09-20 Reply Admin

Hugo:
What an obvious error - how can anyone claim to have >>an extra layer of conformation before doing "serious" things<< if there is only one confirmation required? Sure, we all know that the serious stuff needs at least two, preferably more confirmations: "are you sure" "are you really sure" "are you really really really really really sure" "if you proceed you'll lose important data, please click cancel unless you are definitely sure" etc.
In codeL
Public Sub DeleteAccount(AccountNum As String, Confirm As Boolean, DoubleCheck As Boolean)

 If Confirm <> True Then Exit Sub
If DoubleCheck <> True Then Exit Sub
Dim cmd As New ADODB.Command
 With cmd
 .ActiveConnection = getConn()
 .CommandText = "DELETE_PLAYER_ACCOUNT"
.Parameters.Append _
.CreateParameter("ACCOUNT_NUM", adChar, adParamInput, 8, AccountNum)
 .Parameters.Append _
 .CreateParameter("CONFIRM", adInteger, adParamInput, 4, CInt(Confirm))
 .Parameters.Append _
 .CreateParameter("DOUBLECHECK", adInteger, adParamInput, 4, CInt(DoubleCheck))
 .Execute
 End With
End Sub

If I didn't want it to do anything, I probably just wouldn't call it. Damn wishy-washy invokers!

Murph · 2005-09-20 Reply Admin

If this were a Java shop, the above code might look like this:

public void deleteAccount(String accountNum, paulaBean confirm)
{
   if (paulaBean.getPaula() != "Brillant")
   {
      return;
   }
   ...
}

Clearly you would want to optimize this WTF by replacing the literal Brillant with a static final String because it's faster.

:)

Murph · 2005-09-20 Reply Admin

Sorry... Method should look like this:

public void deleteAccount(String accountNum, paulaBean confirm)
{
   if (confirm.getPaula() != "Brillant")
   {
      return;
   }
   ...
}

2005-09-20 Reply Admin

If confirm is not true or false, we should randomly choose between true or false, since there should be no bias!

Longinus00 · 2005-09-20 Reply Admin

Alex Papadimoulis:

    .ActiveConnection = getConn()
    .CommandText = "DELETE_PLAYER_ACCOUNT"
    .Parameters.Append _
        .CreateParameter("ACCOUNT_NUM", adChar, adParamInput, 8, AccountNum)
    .Parameters.Append _
        .CreateParameter("CONFIRM", adInteger, adParamInput, 4, CInt(Confirm))

Delete player account? Is this from some sort of MMORPG or MUD or something? And seriously, why the cast to int?

Mung Kee · 2005-09-20 Reply Admin

Anonymous:
If confirm is not true or false, we should randomly choose between true or false, since there should be no bias!

Randomly choosing adds a certain mystique to the subroutine. Did it do? Did it not? I just don't know. We'll have to wait for the TPS reports to find out for sure.

CodeToad · 2005-09-20 Reply Admin

Wow, and here I was not bothering to call functions I didn't want to exeute in the first place! I hope I get a blog someday.

icelava · 2005-09-20 Reply Admin

Why, it only makes sense that they pass in the correct result from the user graphical confirmation dialog:

==========
Do you really wish to delete this?
==========
Yes = True
No = False

icelava · 2005-09-20 Reply Admin

Longinus00:
And seriously, why the cast to int?

Who knows if Boolean will have three or four possible values in future.

Gene Wirchenko · 2005-09-20 Reply Admin

Anonymous:
I can think of one semi-plausible reason for doing this. Ever heard the term, "fandango on core?" Though even then, whether this would actually prevent a catastrophe depends entirely on what's on the stack at the time... not to mention that accidentally jumping to the precise address of the DropAllTables_CrashTheProgram_AndSendANastyLetterToYourBoss (bool bConfirm) function is probably really really unlikely in practice, assuming that you don't abuse C++ function pointers while high on paint fumes.

Since all but 0 is true, maybe, that parm should be declared "bool bPleaseDoNot".

Sincerely,

Gene Wirchenko

Gene Wirchenko · 2005-09-20 Reply Admin

MikeB:
If MsgBox("Are your intentions true?",vbYesNo) = vbYes Then
DeleteAccount(AccountNum, True)
Else
DeleteAccount(AccountNum, False)
End if

I don't know this would work for them, but it seems that if you're really serious, you should prompt the user to find out what they think.

Seriously!

"Of course my intentions are true. I want to list the account. What is wrong with you? You idiots in IT are always getting in the way. You turkeys probably think I want to delete the account or something."

Sincerely,

Gene Wirchenko

triso · 2005-09-20 Reply Admin

JohnO:
JohnO:

Anonymous:
Ever heard the term, "fandango on core?"

No and I've been programming for 23 years now.

programming) fandango on core - (Unix/C, from the Mexican dance) In C, a wild pointer that runs out of bounds, causing a core dump, or corrupts the malloc arena in such a way as to cause mysterious failures later on, is sometimes said to have "done a fandango on core". On low-end personal machines without an MMU, this can corrupt the operating system itself, causing massive lossage. Other frenetic dances such as the rhumba, cha-cha, or watusi, may be substituted.

What about the Shag, the Batusi or the Twist? Equal rights for all dances.

Miszou · 2005-09-20 Reply Admin

OneFactor:

It would be really nice to read the daily WTF without comments about performance.

It would be really nice to go to sleep without comments about performance.

2005-09-20 Reply Admin

Oliver Klozoff:
....this one stumps me. I cannot fathom any situation where this code would ever be useful for anything other than Confirm == True.

Unit Tests.

joost · 2005-09-20 Reply Admin

WTF, that so inefficient, imagine how many confirm booleans would be on the stack during an average run. This should obviously be a global boolean.

2005-09-20 Reply Admin

Anonymous:
This might not be soooooo bad if it's called with something like:

DeleteAccount(GetAccountNum("username"), GetConfirmation())

You can wrap it up on one line...though I don't honestly know the benefit (or possible pitfalls) from doing it this way.

I'm pretty sure that's how they use the code, why though? doesn't take much more effort to work that one out. It's so that they don't accidentally forget to confirm the action.

I've seen the following a few times before

if(some statement);
DeleteSomethingImportant();

Of course this could be picked up by some automated process before warning the programmer but it does not guarrantee that a check was done at all.

Why not run the check in the actual procedure? Simple answer again. Separation of the GUI and backend. This might be run via a webinterface or by say a GM using another program.

2005-09-20 Reply Admin

Mung Kee:
Hugo:
What an obvious error - how can anyone claim to have >>an extra layer of conformation before doing "serious" things<< if there is only one confirmation required? Sure, we all know that the serious stuff needs at least two, preferably more confirmations: "are you sure" "are you really sure" "are you really really really really really sure" "if you proceed you'll lose important data, please click cancel unless you are definitely sure" etc.

In codeL
Public Sub DeleteAccount(AccountNum As String, Confirm As Boolean, DoubleCheck As Boolean)

 If Confirm <> True Then Exit Sub
If DoubleCheck <> True Then Exit Sub
Dim cmd As New ADODB.Command
 With cmd
 .ActiveConnection = getConn()
 .CommandText = "DELETE_PLAYER_ACCOUNT"
.Parameters.Append _
.CreateParameter("ACCOUNT_NUM", adChar, adParamInput, 8, AccountNum)
 .Parameters.Append _
 .CreateParameter("CONFIRM", adInteger, adParamInput, 4, CInt(Confirm))
 .Parameters.Append _
 .CreateParameter("DOUBLECHECK", adInteger, adParamInput, 4, CInt(DoubleCheck))
 .Execute
 End With
End Sub
If I didn't want it to do anything, I probably just wouldn't call it. Damn wishy-washy invokers!

And If you want to be 100% certain:

Public Sub DeleteAccount(AccountNum As String, Confirm As Boolean)

If Confirm <> True Then

Exit Sub

else

    for (int i = 1 to 100)

      If not MsgBox("You have called a SERIOUS method. Are you sure you want to proceed?", MsgBoxStyle.OKCancel) = MsgBoxResult.OK Then
        Exit Sub

      end if

    Nex i

end if

Dim cmd As New ADODB.Command
With cmd
.ActiveConnection = getConn()
.CommandText = "DELETE_PLAYER_ACCOUNT" .Parameters.Append _ .CreateParameter("ACCOUNT_NUM", adChar, adParamInput, 8, AccountNum)
.Parameters.Append _
.CreateParameter("CONFIRM", adInteger, adParamInput, 4, CInt(Confirm))
.Parameters.Append _
.CreateParameter("DOUBLECHECK", adInteger, adParamInput, 4, CInt(DoubleCheck))
.Execute
End With End Sub

2005-09-20 Reply Admin

That has an unfair bias against proceeding. You should ask the user if they want to proceed 100 times and record the results. You should also randomly reverse the meaning of the question. This helps to weed out any bias from the user. Some might prefer clicking the right hand button more than the left. Users with submissive personalities may tend towards the Ok button biasing the results.

You can then use statistical analysis to determine if the user wants to proceed.

2005-09-20 Reply Admin

maldrich:

Why, I just realised -- it's not safe at all! That sub could actually run and delete the player's account completely in the background, just by setting Confirm to True!

Here's the safe version:

Public Sub DeleteAccount(AccountNum As String)

  If MsgBox("You have called a SERIOUS method. Are you sure you want to proceed?", MsgBoxStyle.OKCancel) = MsgBoxResult.OK Then
            If MsgBox("Positive?", MsgBoxStyle.OKCancel) = MsgBoxResult.OK Then

  		Dim cmd As New ADODB.Command
  		With cmd
    			.ActiveConnection = getConn()
    			.CommandText = "DELETE_PLAYER_ACCOUNT"
    			.Parameters.Append _
        			.CreateParameter("ACCOUNT_NUM", adChar, adParamInput, 8, AccountNum)
    			.Parameters.Append _
        			.CreateParameter("CONFIRM", adInteger, adParamInput, 4, 1)
    			.Execute
  		End With
      	    End If
  End If
End Sub

Actually, I think you've missed a crucial WTF there:

If MsgBox("Positive?", MsgBoxStyle.OKOnly) <> MsgBoxResult.Cancel Then
   Dim cmd As New ADODB.Command
   With cmd...

dance2die · 2005-09-20 Reply Admin

I really hate it when people do something like

"confirm <> True"

instead of writing simpler and clearer code like

"confirm = False"

Gosh...

2005-09-21 Reply Admin

dance2die:
I really hate it when people do something like

"confirm <> True"

instead of writing simpler and clearer code like

"confirm = False"

Gosh...

I sure hope that was a joke... it should be "confirm == False"

Darquan · 2005-09-21 Reply Admin

This can be usefull if they want to train on the live system.

For example :
DeleteAccount(sMyAccountString, !oMyStaticConfigurationClass.IsTraining)

If the user is in training or simulation mode, the function will not execute the delete, but if the user is in normal usage mode, it will execute.They probably used this in stead of
If(oMyStaticConfigurationClass.IsTraining)
{
return;
}
else
{
DeleteAccount(sMyAccountString);
}

The best thing to do that would avoid all this code is to have the client have a test/training system that would not affect the production system.Mind you the extra system will not work if it is something like Office that you run on a single client, but from this I take it we all can understand what a possible reasoning behind their madness was.

2005-09-21 Reply Admin

JohnO:
JohnO:

Anonymous:
Ever heard the term, "fandango on core?"

No and I've been programming for 23 years now.

programming) fandango on core - (Unix/C, from the Mexican dance) In C, a wild pointer that runs out of bounds, causing a core dump, or corrupts the malloc arena in such a way as to cause mysterious failures later on, is sometimes said to have "done a fandango on core". On low-end personal machines without an MMU, this can corrupt the operating system itself, causing massive lossage. Other frenetic dances such as the rhumba, cha-cha, or watusi, may be substituted.

Yes and the only time I've heard it either is in the Jargon file or people on slashdot trying to sound cool

t-bone · 2005-09-21 Reply Admin

Anonymous:
dance2die:
I really hate it when people do something like

"confirm <> True"

instead of writing simpler and clearer code like

"confirm = False"

Gosh...

I sure hope that was a joke... it should be "confirm == False"

not in VB :)

rikkus · 2005-09-21 Reply Admin

I think I have a valid reason for this code existing: It could have been auto-generated by some kind of interface builder, where you have to tell it to generate a method to run a stored procedure, and drop your AccountNumberTextBox and ConfirmCheckBox on it, telling it that...

Never mind, it's unlikely. It's the ramblings of a drugged horse after all.

DavidK · 2005-09-21 Reply Admin

Anonymous:
JohnO:
JohnO:

Anonymous:
Ever heard the term, "fandango on core?"

No and I've been programming for 23 years now.

programming) fandango on core - (Unix/C, from the Mexican dance) In C, a wild pointer that runs out of bounds, causing a core dump, or corrupts the malloc arena in such a way as to cause mysterious failures later on, is sometimes said to have "done a fandango on core". On low-end personal machines without an MMU, this can corrupt the operating system itself, causing massive lossage. Other frenetic dances such as the rhumba, cha-cha, or watusi, may be substituted.

Yes and the only time I've heard it either is in the Jargon file or people on slashdot trying to sound cool

Ah! Aaaah! But you admit you have heard it then! Ah! Aaaah! Case dismissed.

(I also don't think I've heard it before.... 9 years including enough working with C.)

Back to the topic though... my mind is still bending around this one. It's just nuts. I desperately want to have access to their codebase so that I can check whether anything anywhere ever has the capability of sending False as the value of Confirm. I suspect it has simply never happened.

And to those half-defending it... this is UI logic, it should NOT ever touch the database layer or reach so deep into the code. Think about it! ;)

2005-09-21 Reply Admin

Anonymous:
Anonymous:
Anonymous:
dubwai:

OneFactor:
I'm just waiting for people to post about how today's WTF is so bad because of its performance impact rather than the sheer idiocy behind thinking up such a concoction. Then there will be the "premature optimization is the root of all evil" acolytes putting in their 0010 cents with interest.

It's more like: some anonymous coward will post about how the WTF isn't a WTF because it's faster. Then someone will post about how it's not faster and it is in fact slower. Then you'll have the people saying that not using the WTF for performance reasons is a micro-optimization.

This method of structuring is proven to be faster because if confirm is false it does nothing... hence instance performance boost!

But in fact it's slower since confirm value has to be evaluated.

You can't determine whether the aggregate is faster or slower without profiling, because you don't know what proportion of calls to this function have confirm set to true, what proportion have it set to false, and what proportion have it set to SchrodingerSCat. This is only a WTF if the programmer didn't profile properly before and after adding this optimization.

Congrats, you win the argument.

also read: Zen of Code Optimizing, Michael Abrash (c) somewhere in the middle-ages of programming.

TINSTATFC.

2005-09-21 Reply Admin

JohnO:

...causing massive lossage....

Lossage?

Kefer · 2005-09-21 Reply Admin

icelava:
Longinus00:
And seriously, why the cast to int?
Who knows if Boolean will have three or four possible values in future.

Three or four? Even quite a lot more: it's a 4 byte int...

brazzy · 2005-09-21 Reply Admin

Anonymous:

Yes and the only time I've heard it either is in the Jargon file or people on slashdot trying to sound cool

...who read it in the Jargon file. It's a general problem with any attempt to document slang: it's rather fashion-dependent. Some of it persists and spreads into mainstream usage, some falls out of usage completely. There was a time when the word "spam" for junk email was just as obscure...

dhromed · 2005-09-21 Reply Admin

joost:
WTF, that so inefficient, imagine how many confirm booleans would be on the stack during an average run. This should obviously be a global boolean.

Superb idea!

Dim UtterlyDisableApplication As Boolean
UtterlyDisableApplication = True

And this WTF is perfectly acceptable in my book. I mean, I keep an empty bottle in the fridge for when visitors don't want anything to drink.

dhromed · 2005-09-21 Reply Admin

My smarts caught up with myself.

Should, of course, be:

Dim EnableApplication As Boolean

EnableApplication = False

Syarzhuk · 2005-09-21 Reply Admin

RayS:
What I'm sure we can all agree on though is that whoever mandated this 'security' feature needs to have the 2nd half of his brain removed to put in the jar with the 1st half.

This is impossible, because .jar is a Java format and this is VB coder we're talking about. VBX would do just fine, though.

Syarzhuk · 2005-09-21 Reply Admin

travisowens:
I think the previous post said it best about sending a user & pass with the method, now you can really prevent people from executing methods unless they look up the user/pass in the code. And imagine the coding hell it will create when you need to change the user/pass oneday! My evil mind is in bliss as we speak!

No coding hell if you use proper development techniques, specifically in this case - named constants. Refactor the previous poster's code to:

Global PROPER_USER_NAME = "programmer"

Global PROPER_PASSWORD = "initech"

Function Add( a, b, user, password )
if user=PROPER_USER_NAME and password=PROPER_PASSWORD then

Add = a + b
else
Add = 0 ' unauthorized programmer, will get bad result
End Function

That was, if the password ever changes, you only need to change it in one place and no coding hell ensures

Generic · 2005-09-21 Reply Admin

Anonymous:
Generic:
Alex Papadimoulis:
.ActiveConnection = getConn()
WTF! It shuld be:
Set .ActiveConnection = getConn()
Else you use a second connection object. Because the code assigne the string value from and to the default property of the connection object. The ConnectionString property.
I think it's VB.NET, "set" keyword is obsolete.

I know that set is not used in VB.NET.

But the code looks like old classic VB 5/6 code. I know my enemy. Seems like you se .NET everywhere.

Syarzhuk · 2005-09-21 Reply Admin

MikeB:

If MsgBox("Are your intentions true?",vbYesNo) = vbYes Then
DeleteAccount(AccountNum, True)
Else
DeleteAccount(AccountNum, False)
End if

Which obviously should be refactored to a shorter and more readable version:

DeleteAccount(AccountNum, MsgBox("Are your intentions true?",vbYesNo) = vbYes)

This way you reduce the complexity and make the code easier to maintain.

JohnO · 2005-09-21 Reply Admin

Anonymous:
dance2die:
I really hate it when people do something like

"confirm <> True"

instead of writing simpler and clearer code like

"confirm = False"

Gosh...
I sure hope that was a joke... it should be "confirm == False"

I sure hope that was a joke it should be either:

!confirm //c#

or

not confirm //vb

Alexis de Torquemada · 2005-09-21 Reply Admin

Gene Wirchenko:
Anonymous:
I can think of one semi-plausible reason for doing this. Ever heard the term, "fandango on core?" Though even then, whether this would actually prevent a catastrophe depends entirely on what's on the stack at the time... not to mention that accidentally jumping to the precise address of the DropAllTables_CrashTheProgram_AndSendANastyLetterToYourBoss (bool bConfirm) function is probably really really unlikely in practice, assuming that you don't abuse C++ function pointers while high on paint fumes.

Since all but 0 is true, maybe, that parm should be declared "bool bPleaseDoNot".

Sincerely,

Gene Wirchenko

in C(++):

int imTotallyAndUtterlySure = 0xDEADF00D;

...
if (confirm != imTotallyAndUtterlySure)
return;

Corkscrew · 2005-09-21 Reply Admin

Syarzhuk:
travisowens:
I think the previous post said it best about sending a user & pass with the method, now you can really prevent people from executing methods unless they look up the user/pass in the code. And imagine the coding hell it will create when you need to change the user/pass oneday! My evil mind is in bliss as we speak!

No coding hell if you use proper development techniques, specifically in this case - named constants. Refactor the previous poster's code to:

Global PROPER_USER_NAME = "programmer"

Global PROPER_PASSWORD = "initech"

Function Add( a, b, user, password )
if user=PROPER_USER_NAME and password=PROPER_PASSWORD then

Add = a + b
else
Add = 0 ' unauthorized programmer, will get bad result
End Function

That was, if the password ever changes, you only need to change it in one place and no coding hell ensures

Isn't Global deprecated in VB?

2005-09-21 Reply Admin

> The key question to me is why would you ever pass false for the "confirm" parameter?

As a delay? >:)

Alexis de Torquemada · 2005-09-21 Reply Admin

Anonymous:
That has an unfair bias against proceeding. You should ask the user if they want to proceed 100 times and record the results. You should also randomly reverse the meaning of the question. This helps to weed out any bias from the user. Some might prefer clicking the right hand button more than the left. Users with submissive personalities may tend towards the Ok button biasing the results. You can then use statistical analysis to determine if the user wants to proceed.

Excellent approach, but the position of the OK and Cancel buttons should also be randomized, just in case. And I think checking only 100 times may be a bit too lenient. After all, it's really important data, isn't it?

Foreign Methodologies

Leave a comment on “Foreign Methodologies”