Comment On XML, Recursion, and ... VBScript?

Alex Papadimoulis:

    

  Select Case True

      ....

    Case Else

  End Select

    

What happens when True evaluates to false? There's no default code!!

John Smallberries:

Alex Papadimoulis:

    

  Select Case True

      ....

    Case Else

  End Select

    

What happens when True evaluates to false? There's no default code!!

All programmers who put hundreds of lines of code in branches of a conditional statement deserve to be put to the torch!

Not to mention thousands of lines in a method.

Grrr.

Common sLogic says the 1nTrueRuleID is that you must always bTrue to yourself, no matter what the nRuleIndex might be. That, my friends, will never bFalse.

 

 

 

    If nTrueRuleID <> 0 Then
      bTrue = True
      Call ProcessRule(1, nTrueRuleID, sLogic)
    Else
      If nFalseRuleID <> 0 Then
        bFalse = True
        Call ProcessRule(2, nFalseRuleID, sLogic)
        Exit Sub
      End If
    End If
        
    If nFalseRuleID <> 0 Then
      bFalse = True
      Call ProcessRule(2, nFalseRuleID, sLogic)
    End If

Nice way to abuse "early return". Notice the redundant code below the block that contains the exit sub.



Even Nicer:

  If nTrueRuleID = 0 And nFalseRuleID = 0 Then
    'Snipped: 1500 lines of business-specific logic
    Exit Sub
  Else

Either the author of this abortion had no clue, or didn't have any respect for maintenence programmers. Or a bit of both.

Ha! There's actually well know pattern (or anti-pattern) that
identifies this: The Blob (AKA The God Class). Although not a class, it
more than takes up the lion share of responsibilities for the entire
system. The Blob is a god awful code structure and impossible to
maintain... whenever I see one in code I have to work on, I instantly
break it up into smaller peices to handle specific things, then
refactor from there.



But good god.. that is the worst thing I have ever seen... in VBScript no less :(  





Also, amusing how this site mocks other people's code, yet the code for
this system is a wtf! I have entered the CAPTCH correctly twice now and
it still tells me it didn't validate... WTF!?

Anonymous:

Also, amusing how this site mocks other people's code, yet the code for
this system is a wtf! I have entered the CAPTCH correctly twice now and
it still tells me it didn't validate... WTF!?

The first captcha always fails (at least this is what I experienced).
The second captcha will work, but only if you enter it in ALL CAPS (no
matter whether the displayed text is actually in caps). HTH.

There must be some missing variables. I see nTrueRuleID and
nFalseRuleID, but where are nPartiallyTrueID, nMostlyFalseID,
nReallyDontKnowId?

Alex Papadimoulis:

  Dim bTrue           As Boolean

  Dim bFalse          As Boolean

   

I've seen tons of garbage "code" on this site but I am really left scratching my head after reading this one.


Where did the "programmer" come up with the idea of declaring the variables bTrue and bFalse?!?!

What was his thought process when he decided to do this?

ferrengi:

Where did the "programmer" come up with the idea of declaring the variables bTrue and bFalse?!?!

What was his thought process when he decided to do this?

What was what?

So this guy spent weeks rewriting a module that does nothing different from the original?  There's your WTF.

Alex Papadimoulis:

Private Sub ProcessRule(nType As Long, nRuleIndex As Long, ByRef sLogic As String)

  Const METHOD_NAME = "GetApplicationVariable"



  Dim bTrue           As Boolean

  Dim bFalse          As Boolean

   

  If nType = 1 Then

    'Root Level, retreive parameters ...

  Else

    Call ProcessRule(nType - 1, nRuleIndex, sLogic)

    Exit Sub

  End If

 

I don't get it. 

One, if  process rule gets called with nType < 1, it will go to never never land, only to be rescued by some kind of stack error.

Two, what's the point of ever calling it with anything but nType = 1?  Maybe the OP really liked a busy vb call stack window when debugging?

 

Alexis de Torquemada:

ferrengi:

Where did the
"programmer" come up with the idea of declaring the variables bTrue and
bFalse?!?!

What was his thought process when he decided to do this?

What was what?

Yeah, good point.

My question assumes that he was actually thinking when he did this.

Anonymous:

Alex Papadimoulis:

Private Sub ProcessRule(nType As Long, nRuleIndex As Long, ByRef sLogic As String)
  Const METHOD_NAME = "GetApplicationVariable"



  Dim bTrue           As Boolean

  Dim bFalse          As Boolean

   

  If nType = 1 Then

    'Root Level, retreive parameters ...

  Else

    Call ProcessRule(nType - 1, nRuleIndex, sLogic)
    Exit Sub

  End If

 

I don't get it. 

One, if  process rule gets called with nType < 1, it will go
to never never land, only to be rescued by some kind of stack error.

Two, what's the point of ever calling it with anything but nType =
1?  Maybe the OP really liked a busy vb call stack window when
debugging?

 

1) It won't go to never never land if nType < 1 because the confirm variable has not been set to bTrue



2) There is no point. This method is so well written that it will
execute properly (eventually and provided that nType is a positive
number) using recursion. So the programmer calling this method need not
worry about passing the correct value for nType.

Anonymous:

So this guy spent weeks rewriting a module that does nothing different from the original?  There's your WTF.

Apparently you are unaware of how costly it is to maintain poorly written code.

John Smallberries:

Alex Papadimoulis:

    

  Select Case True

      ....

    Case Else

  End Select

    

What happens when True evaluates to false? There's no default code!!

Anonymous:

So this guy spent weeks rewriting a module that
does nothing different from the original?  There's your WTF.

No. The idea is that afterwards you have a module that has the same
functionality AND is maintainable so that it can be extended or
modified when needed without the application breaking down in twenty
unexpected ways.



Of course, it would probably have been a lot easier and quicker to just
throw away the junk and reimplement the specification from scratch...
except that there apparently WAS NO specification.

It's been my experience that this code is worthless unless it has bContinue.



Trust me.

what's the point of passing a variable in, checking if it's not 1, then RECURSIVELY subtracting 1?  First off, if you want the value to = 1 when you're done iterating, just say

if variable != 1 then variable = 1

aaaaaand, if the variable is always going to be equal to 1 (because you said so), why not just like.....not worry about ever using it.  Or make it a constant.  SOMETHING!

that last chunk of code truly baffles me.  I wanna know who taught this person to name your variables bTrue and bFalse.  Can bTrue = bFalse ?  If a tree falls in a forest and there's nobody there to hear it, does it make a sound?  that's some deep ass shit right there.......

So what happens when True is False and False is True?  Smart becomes Dumb and Dumb becomes Smart?

And I think he forgot the everso critical bNotTrue and bNotFalse

I think I have figured out the captcha "problem".  If you click
the button, it works, if you hit return to continue to the next page,
the return character is added to whatever you typed, resulting in an
invalid captcha test.  Anyone else want to verify?

Just signup for the site... no captcha required.

brazzy:

Anonymous:

So this guy spent weeks rewriting a module that does nothing different from the original?  There's your WTF.

No. The idea is that afterwards you have a module that has the same functionality AND is maintainable so that it can be extended or modified when needed without the application breaking down in twenty unexpected ways.

Of course, it would probably have been a lot easier and quicker to just throw away the junk and reimplement the specification from scratch... except that there apparently WAS NO specification.

And tell me, who the hell will grant that the current module, that is being written CAN be mainteable? Huh? Huh? Who will grant that? Because he sent the old code to TDWTF? Does that make him a wtf-proof programmer? Everybody that posts on this site never written a wtf?
That's an wtf in itself. 
Re-writing the same code to do the same thing. Unless someone will make sure it's mainteable. Now, who will grant that?

ferrengi:

Alex Papadimoulis:

  Dim bTrue           As Boolean

  Dim bFalse          As Boolean

   

I've seen tons of garbage "code" on this site but I am really left scratching my head after reading this one.
Where did the "programmer" come up with the idea of declaring the variables bTrue and bFalse?!?!
What was his thought process when he decided to do this?

Even worse:

bFalse = True

Thank you for your insights in deep programming knowledge, this one really made my day.

I will have to go rewrite all of the code I produced so far, but damn, this is worth it.

I'm surprised no one has pointed out this part yet...

Alex Papadimoulis:

Private Sub ProcessRule(nType As Long, nRuleIndex As Long, ByRef sLogic As String)

  Const METHOD_NAME = "GetApplicationVariable"

I assume he's using the METHOD_NAME in the error handler.

So good luck tracking down any run-time errors that happen in this function.

I'll admit not knowing anything of VB... but... is it me or he's recursively calling his function until he gets to "nType==1" to do his things, doing nothing whatsoever every time said nType is différent that one?

As in he could have actually *not* passed the nType parameter and could have gotten rid of all those recursions?

A Wizard A True Star:

I'm surprised no one has pointed out this part yet...

Alex Papadimoulis:

Private Sub ProcessRule(nType As Long, nRuleIndex As Long, ByRef sLogic As String)

  Const METHOD_NAME = "GetApplicationVariable"

I assume he's using the METHOD_NAME in the error handler.

So good luck tracking down any run-time errors that happen in this function.

dubwai:

Anonymous:

So this guy spent weeks
rewriting a module that does nothing different from the original? 
There's your WTF.

Apparently you are unaware of how costly it is to maintain poorly written code.

Oh I completely understand how frustrating it can be to look at crap
code.  I'm thinking more in a business sense.  Its not easy
to justify the expense of a rewrite unless specific defects are
addressed or additional functionality is introduced. (i.e. have
something to show for it).    If you can point to the
code and say, "Yeah, all this is going to explode if x, y, and z
happen", then yes, it is justified.  But if you just don't like
the (lack of) design, or the (lack of) coding standards, or anything
which has no bearing on the actual use of the product, then you're just
wasting money.  

Anonymous:

All programmers who put hundreds of lines of code in branches of a conditional statement deserve to be put to the torch! Not to mention thousands of lines in a method. Grrr.

 

Exactly, what the hell do you think gosubs are for!

Anonymous:

dubwai:

Anonymous:

So this guy spent weeks
rewriting a module that does nothing different from the original? 
There's your WTF.

Apparently you are unaware of how costly it is to maintain poorly written code.

Oh I completely understand how frustrating it can be to look at crap
code.

I think you failed to noticed the part where he mentioned the word maintain.

Oh, and rewriting that kind of code before it actually breaks is actually considered being some kind of... like... good practice for future safety

Anonymous:

But if you just don't like
the (lack of) design, or the (lack of) coding standards, or anything
which has no bearing on the actual use of the product, then you're just
wasting money.  

Take a long term view of the problem:  What if spending three weeks rewriting it right now saves you three years
in the future, because the new module can be modified much more
quickly?  In that case, you actually saved nearly three years, not
wasted three weeks.  The danger is that you could spend three weeks and then never touch it again, in which case you wasted three weeks and saved zero hours.

loneprogrammer:

What if spending three weeks rewriting it right now saves you three years
in the future . . .

I do not mean three years in one lump sum.  I mean, e.g. 2 months
here, 3 months there, and so on . . .  Sometimes it is easier to
take some pain all at once than to do a "quick and dirty" job over and
over and over again.

masklinn:

I think you failed to noticed the part where he mentioned the word maintain.

Oh, and rewriting that kind of code before it actually breaks is actually considered being some kind of... like... good practice for future safety

Yes, to maintain code you must look at it.  That's the hard part
about maintaining someone else's code.  Figuring out what they're
doing.



Do you think it is a good practice to bill your customers thousands of
dollars to rewrite software that you initially sold them because you
think it's crap?  

Anonymous:

masklinn:

I think you failed to noticed the part where he mentioned the word maintain.

Oh, and rewriting that kind of code before it actually breaks is actually considered being some kind of... like... good practice for future safety

Yes, to maintain code you must look at it.  That's the hard part about maintaining someone else's code.  Figuring out what they're doing.

Do you think it is a good practice to bill your customers thousands of dollars to rewrite software that you initially sold them because you think it's crap?  

Yes. Good practise for my bank account.

However, if you'd read the original post, you'd have noticed this

Steve F works at a large financial institution as a .NET programmer was excited to finally get a chance to use his skills and rewrite a VB6 modules.

He's not the contractor that sold them the initial crap.

Anonymous:

And tell me, who the hell will grant that the current module, that is being written CAN be mainteable? Huh? Huh? Who will grant that? Because he sent the old code to TDWTF? Does that make him a wtf-proof programmer? Everybody that posts on this site never written a wtf?
That's an wtf in itself. 
Re-writing the same code to do the same thing. Unless someone will make sure it's mainteable. Now, who will grant that?

Noone can be sure that it will be maintainable, but the current code is not. Making changes to this code would be impossible.

 I've seen many cases of huge VB apps that grown from a utility to a business critical services, and then changes become increasingly risky and faulty. *IF* its rewritten with a more maintainable language, (.net perhaps) then future changes become cheapers and more reliable.

Dim bTrue As Boolean

Dim bFalse As Boolean

My eyes!  The goggles do nothing!!!!!!

Mike:

Oh I completely understand how frustrating it can be to look at crap
code.  I'm thinking more in a business sense.  Its not easy
to justify the expense of a rewrite unless specific defects are
addressed or additional functionality is introduced. (i.e. have
something to show for it).    If you can point to the
code and say, "Yeah, all this is going to explode if x, y, and z
happen", then yes, it is justified.  But if you just don't like
the (lack of) design, or the (lack of) coding standards, or anything
which has no bearing on the actual use of the product, then you're just
wasting money.  

If the code is totally uncomprehensible, as in this example, nobody can
be sure if it works correctly. A software product that is meant to be
more than a toy is practially worthless if you have no idea what it
does, how it does it and under what conditions. Even if it appears to
work, it's a time-bomb. You cannot tell what x,y and z in "Yeah, all
this is going to explode if x, y, and z
happen" are.

Cyresse:

Steve F. works at a large financial institution as a .NET programmer was excited to finally get a chance to use his skills and rewrite a VB6 modules.

He's not the contractor that sold them the initial crap.

True, but even Steve's department has a "customer", most likely another business unit within the company.  Depending on how the accounting is set up, the the cost of the rewrite may be billed to the client department.

No one noticed that:

Unfortunately, since no one really understood how the existing module worked or even all that it did, the spec was basically to make sure the new component acted like the old one.

Emphasis on "make sure the new component acted like the old one".
Should it also replicate the bugs when writing the new version?

Anonymous:

All programmers who put hundreds of lines of
code in branches of a conditional statement deserve to be put to the
torch!
Not to mention thousands of lines in a method.
Grrr.

More accurately, all programmers who leave
it that way.  I sometimes write a block of code in straight
top-to-bottom fashion, then promptly go back and refactor the longer
chunks into subroutines.

Anonymous:

So this guy spent weeks rewriting a module that does nothing different from the original?  There's your WTF.

Er, no, this WAS the original code. Just now he sorta knows what the core of it sorta does.      

Anonymous:

So this guy spent weeks rewriting a module that does nothing different from the original?  There's your WTF.

Changed the "how" not the "what" the program was doing, as per the requirements.

Anonymous:

Do you think it is a good practice to bill your customers thousands of
dollars to rewrite software that you initially sold them because you
think it's crap?  

Note that this is not the case here since it's an employee of the
customer doing the rewrite. Slowly and painfully because he first has
to understand the crap code without any specs.



And this is exactly what an earlier rewrite by the people who
originally did it (and therefore should be able to rewrite it much more
quickly) would have SAVED the customer - a lot of time and money.



Of course, the person who conceived this idiocy could obviously not
write good code to save his life, but he could have explained what he
did to another, hopefully competet, coder.

user="Alex Papadimoulis"

   



  If nType = 1 Then

    'Root Level, retreive parameters ...

  Else

    Call ProcessRule(nType - 1, nRuleIndex, sLogic)

    Exit Sub

  End If

I assume that nType did something when the routine was written for the first time, or at least was intended to be a very clever way to do something.

Later on, the programmer must have noticed that recursity is useless for almost anything :) and then he wrote the enormous Select Case section that only took into account nRuleIndex and sLogic.

Then, because all the code in the system must have been littered with calls to the procedure that used nType, he couldn't take out the nType parameter without breaking half the applications in the bussines, so he simply deleted the cases for nType==2, nType==3, etc.

One of the WTFs is not rewriting the recursive code to some simple assignment like nType=1, and then proceeding as usual. Maybe he thought that changing from recursive to procedural could break legacy code? :) Do electric sheeps mind that they dream using recursive or using procedural functions, as long as they dream the same dreams?

Alex Papadimoulis:

  Select Case True

    Case bTrue And bFalse

      'SNIP: 100's of lines of business logic

    Case (bTrue) And (Not bFalse)

      'SNIP: 100's of lines of business logic

    Case (Not bTrue) And (bFalse)

      'SNIP: 100's of lines of business logic

    Case (Not bTrue) And (Not bFalse)

      'SNIP: 100's of lines of business logic

    Case Else

  End Select

Of course, it's clear that the Case Else should never have been left empty. Here's my suggested fix:

  Select Case True

    Case bTrue And bFalse

      'SNIP: 100's of lines of business logic

    Case (bTrue) And (Not bFalse)

      'SNIP: 100's of lines of business logic

    Case (Not bTrue) And (bFalse)

      'SNIP: 100's of lines of business logic

    Case (Not bTrue) And (Not bFalse)

      'SNIP: 100's of lines of business logic

    Case Else
      'Continue trying until something is processed
      Do While True
        'Randomly select one of the four possibilities because there should be no bias.
        Select Case Random(4)
          Case 1

            'Verbatim copy of first 100's of lines of snipped business logic

            Exit Do

          Case 2

            'Verbatim copy of second 100's of lines of snipped business logic

            Exit Do

          Case 3

            'Verbatim copy of third 100's of lines of snipped business logic

            Exit Do

          Case 4

            'Verbatim copy of fourth 100's of lines of snipped business logic

            Exit Do
          Case Else
        End Select
      Loop

  End Select

Of course, if you dislike the Do While True construction, you could also nest the Case expression a couple of times. I think that nesting to 5 levels should be adequate.

Oh, and please don't make the mistake of moving the four sets of 100's of lines of snipped business logic to a seperate Sub that can be called from the various Case clauses. Everyone knows that maintaining code is needlessly complicated when you have to check more than one Sub at the same time. Plus, the overhead of calling another Sub and returning when done would kill performance.

Alexis de Torquemada:

The first captcha always fails (at least this is what I experienced).
The second captcha will work, but only if you enter it in ALL CAPS (no
matter whether the displayed text is actually in caps). HTH.

I actually think it's a timing issue. The captcha is generated when the
edit page is displayed, and I suppose it has a timeout after which it
is no longer valid. If you manage to type out your post in that
timespan (which seems to be only a few seconds), it would be accepted.

Anonymous:

No one noticed that:

Unfortunately, since no one really understood how the existing module worked or even all that it did, the spec was basically to make sure the new component acted like the old one.

Emphasis on "make sure the new component acted like the old one".
Should it also replicate the bugs when writing the new version?

Well, that in itself doesn't have to be bad practice. What looks like a bug to a programmer could be some weird but intended functionality. Or the users are so accustomed to working around it that you can't remove the bug without informing the users first. Or removing the bug might break some other program that relies on this bug (or that's even written to counteract the effects - yes, those things do happen, especially with nigh on undecipherable legacy code).

Some time ago (no, actually a long time ago - I'm talking PL/1 on a mainframe computer, that long ago!!), I was given a similar task. When I was done, I delivered three deliverables:

1. Rewritten code that did exactly what the old code did (only with much less lines of code, much more comments, better structured and performing about twice as fast);

2. A complete set of documentation describing what the program actually did (including all the things I suspected to be bugs);

3. A seperate list of only the functionality that I suspected to be bugs, to be reviewed by end users and specialists, so that they could decide if exploding when x, y, and z happens was indeed intended behaviour or should be removed.

Most of the things in deliverable #3 were indeed bugs, and were easily fixed. Some were intended behaviour, so I left them in and added some more comments for my successor.

Hugo:

Of course, it's clear that the Case Else should never have been left empty. Here's my suggested fix:

(snip)

Of course, if you dislike the Do While True construction, you could
also nest the Case expression a couple of times. I think that nesting
to 5 levels should be adequate.

Oh, and please don't make the mistake of moving the four sets of
100's of lines of snipped business logic to a seperate Sub that can be
called from the various Case clauses. Everyone knows that maintaining
code is needlessly complicated when you have to check more than one Sub
at the same time. Plus, the overhead of calling another Sub and
returning when done would kill performance.

brillant!!!

i get it. The WTF is that the 'method_name' is wrong