- Feature Articles
- CodeSOD
- Error'd
- Forums
-
Other Articles
- Random Article
- Other Series
- Alex's Soapbox
- Announcements
- Best of…
- Best of Email
- Best of the Sidebar
- Bring Your Own Code
- Coded Smorgasbord
- Mandatory Fun Day
- Off Topic
- Representative Line
- News Roundup
- Editor's Soapbox
- Software on the Rocks
- Souvenir Potpourri
- Sponsor Post
- Tales from the Interview
- The Daily WTF: Live
- Virtudyne
Admin
What happens when True evaluates to false? There's no default code!!
Admin
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
Admin
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.
Admin
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.
Admin
Nice way to abuse "early return". Notice the redundant code below the block that contains the exit sub.
Even Nicer:
Either the author of this abortion had no clue, or didn't have any respect for maintenence programmers. Or a bit of both.
Admin
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!?
Admin
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.
Admin
There must be some missing variables. I see nTrueRuleID and nFalseRuleID, but where are nPartiallyTrueID, nMostlyFalseID, nReallyDontKnowId?
Admin
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?
Admin
What was what?
Admin
So this guy spent weeks rewriting a module that does nothing different from the original? There's your WTF.
Admin
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?
Admin
Yeah, good point.
My question assumes that he was actually thinking when he did this.
Admin
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.
Admin
Apparently you are unaware of how costly it is to maintain poorly written code.
Admin
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
Admin
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.
Admin
It's been my experience that this code is worthless unless it has bContinue.
Trust me.
Admin
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.......
Admin
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
Admin
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?
Admin
Just signup for the site... no captcha required.
Admin
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?
Admin
Even worse:
bFalse = True
Admin
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.
Admin
I'm surprised no one has pointed out this part yet...
Admin
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?
Admin
Good luck period!
Admin
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.
Admin
Exactly, what the hell do you think gosubs are for!
Admin
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
Admin
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.
Admin
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.
Admin
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?
Admin
Yes. Good practise for my bank account.
However, if you'd read the original post, you'd have noticed this
He's not the contractor that sold them the initial crap.
Admin
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.
Admin
My eyes! The goggles do nothing!!!!!!
Admin
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.
Admin
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.
Admin
No one noticed that:
Emphasis on "make sure the new component acted like the old one".
Should it also replicate the bugs when writing the new version?
Admin
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.
Admin
Er, no, this WAS the original code. Just now he sorta knows what the core of it sorta does.
Admin
Changed the "how" not the "what" the program was doing, as per the requirements.
Admin
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.
Admin
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?
Admin
Of course, it's clear that the Case Else should never have been left empty. Here's my suggested fix:
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.
Admin
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.
Admin
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.
Admin
brillant!!!
Admin
i get it. The WTF is that the 'method_name' is wrong