• John Smallberries (cs)
    Alex Papadimoulis:
        
      Select Case True
          ....
        Case Else
      End Select
    


    What happens when True evaluates to false? There's no default code!!
  • Anonymous (unregistered) in reply to John Smallberries
    John Smallberries:
    Alex Papadimoulis:
        
      Select Case True
          ....
        Case Else
      End Select
    


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

    That's why they should've used notorious isTrue() function.

      Select Case isTrue(True) ....
        Case Else
      End Select
       

  • Eric the .5b (unregistered)

    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.

  • Dave (unregistered)

    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.

     

     

     

  • Mike R (cs)

        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.
  • JC (unregistered)

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

  • Alexis de Torquemada (cs) in reply to JC
    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.

  • CornedBee (cs)

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

  • ferrengi (cs)
    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?

  • Alexis de Torquemada (cs) in reply to ferrengi
    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?
  • Mike (unregistered)

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

  • Rastafas (unregistered)
    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?

     

  • ferrengi (cs) in reply to Alexis de Torquemada
    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.
  • ferrengi (cs) in reply to Rastafas
    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.
  • dubwai (cs) in reply to Mike

    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.

  • Maurits (cs) in reply to John Smallberries
    John Smallberries:
    Alex Papadimoulis:
        
      Select Case True
          ....
        Case Else
      End Select
    


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


    Actually

    Select Case True
        Case condition1
           block1
        Case condition2
           block2
        ...
        Case Else
           block0
    End Select

    is a not-terribly-uncommon way to rewrite

    If condition1 Then
        block1
    ElseIf condition2 Then
        block2
    ...
    Else
        block0
    End If

  • brazzy (cs) in reply to Mike
    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.

  • christoofar (unregistered) in reply to John Smallberries

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

    Trust me.

  • ItsAllGeekToMe (cs)

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

  • Kaizer (cs)

    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

  • Disgruntled DBA (unregistered)

    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?

  • Kaizer (cs) in reply to Disgruntled DBA

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

  • My name (unregistered) in reply to brazzy

    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?

  • . (unregistered) in reply to ferrengi
    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

  • badong (unregistered) in reply to ferrengi

    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.

  • A Wizard A True Star (cs)

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

    Alex Papadimoulis:

    Private Sub <FONT style="BACKGROUND-COLOR: #ffff00">ProcessRule</FONT>(nType As Long, nRuleIndex As Long, ByRef sLogic As String)
      Const METHOD_NAME = "<FONT style="BACKGROUND-COLOR: #ffff00" color=#808080>GetApplicationVariable</FONT>"
    
    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.
     
  • masklinn (cs) in reply to A Wizard A True Star

    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?

  • Anonymous (unregistered) in reply to A Wizard A True Star
    A Wizard A True Star:

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

    Alex Papadimoulis:

    Private Sub <FONT style="BACKGROUND-COLOR: #ffff00">ProcessRule</FONT>(nType As Long, nRuleIndex As Long, ByRef sLogic As String)
      Const METHOD_NAME = "<FONT style="BACKGROUND-COLOR: #ffff00" color=#808080>GetApplicationVariable</FONT>"
    
    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.
     

    Good luck period!

  • Mike (unregistered) in reply to dubwai
    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.  
  • Anonymouse Cowherd (unregistered) in reply to Eric the .5b

    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!

  • masklinn (cs) in reply to Mike
    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

  • loneprogrammer (cs) in reply to Mike
    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 (cs) in reply to loneprogrammer
    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.
  • Mike (unregistered) in reply to masklinn
    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?  
  • Cyresse (cs) in reply to Mike
    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.

  • Filthysock (cs) in reply to My name

    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.

  • JimNtexas (cs)

    Dim bTrue As Boolean
    Dim
    bFalse As Boolean

    My eyes!  The goggles do nothing!!!!!!
  • ammoQ (cs) in reply to Mike
    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.
  • Mike (unregistered) in reply to Cyresse
    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.
  • My Name (unregistered)

    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?

  • emurphy (cs) in reply to Eric the .5b
    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.

  • Dave (unregistered) in reply to Mike

    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.      

  • a (unregistered) in reply to Mike
    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.

  • brazzy (cs) in reply to Mike
    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.
  • Enric Naval (cs)
    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?

  • Hugo (cs)
    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
    <FONT color=#000000> </FONT>'Continue trying until something is processed
    <FONT color=#0000ff> Do While True</FONT>
    <FONT color=#000000> </FONT>'Randomly select one of the four possibilities because there should be no bias.
    Select Case Random(4)
    Case 1 <FONT color=#000000> </FONT>'Verbatim copy of first 100's of lines of snipped business logic
    <FONT color=#0000ff> Exit Do Case 2</FONT> 'Verbatim copy of second 100's of lines of snipped business logic <FONT color=#0000ff> Exit Do Case 3</FONT> 'Verbatim copy of third 100's of lines of snipped business logic <FONT color=#0000ff> Exit Do Case 4</FONT> 'Verbatim copy of fourth 100's of lines of snipped business logic <FONT color=#0000ff> Exit Do
    Case Else
    End Select
    Loop</FONT><FONT color=#0000ff> </FONT> 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.

  • Anonymoth (unregistered) in reply to Alexis de Torquemada
    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.
  • Hugo (cs) in reply to My Name
    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.

  • ammoQ (cs) in reply to Hugo
    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!!!
  • mockney piers (unregistered)

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

Leave a comment on “XML, Recursion, and ... VBScript?”

Log In or post as a guest

Replying to comment #:

« Return to Article