- Feature Articles
- CodeSOD
- Error'd
-
Forums
-
Other Articles
- Random Article
- 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
This is exactly what Dijkstra was talking about...
Admin
That ASSERT is priceless.
Admin
Especially because of the comment behind it, because the writer partially sees how dumb he is (but it makes him look even dumber :P).
Admin
Man, if someone showed me that code on an interview, I'd just go home.
Admin
I'll take mine with a Vodka sauce
Admin
<font size="2">My favorite part is the fact that he's doing a do loop instead of a
while so he has to check for an empty list inside (why he didn't just
check it before entering the loop is another WTF). If he just
changed that around to:</font>
<font size="2"></font>
<font size="2">while (!IsListEmpty(List))</font>
<font size="2"></font>
<font size="2">then he wouldn't need</font>
<font size="2"></font>
<font size="2">
</font><font size="2">
Of course the real WTF here is the goto since all it serves to do is jump him past his own ASSERT!</font>
<font size="3">
</font>
Admin
OK, so does anyone see any reason why the follow should work the same way:
It maintains every possible limitation I was able to derive from the original code, namely:
I actually, doesn't think 1 & 2 are actual requirements, but can't tell without the rest of the source code (#3, on the other hand, I feel is quite legitimate). If we remove them, we get:
Admin
Also he checks it in the while condition at the end, so the inner if isn't necessary, even with the rest of the stupid crap here.
I don't understand the thought pattern that leads to this kind of code. Was this guy perhaps in circulation from before structured programming became the dogma?
Admin
Actually, that has nothing to do with it. The if/goto is completely supurfluous. Presumably, he would have reached this code, unless there was at least one entry to work on.
Admin
What I really don't get about this is that there is so much more work (and thought) involved to do this intead of the obvious and short way. Is it just me or do some programmers think that writing arcane code makes them better programmers? Like it's a one-up for them if someone struggles to get their head around their code regardless of how mundane it really is. I'll be honest, it took me a minute to figure out just what the hell this code was doing and I have to maintain garbage code all the time.
Admin
This is my favorite kind of WTF: the time-release WTF. Like the above poster, I had to stare for a while before I saw what the author was trying to do, and then came a steady stream of realizations as to how bad this code really is. It doesn't hit you all at once; it's like drinking fine whiskey instead of chugging shots.
Admin
But even as bad as that is, it still doesn't explain the real WTF, the totally useless goto statement.
Admin
I heard about these so called "veteran programmers" from an older programmer at the office here. Apparently, back in the day, it was very popular to purposely obfuscate code using gotos and horrible while loops, to obtain some sort of "1337" status. And so these old farts still program the same way to this day, completely ignoring common code conventions, like... I don't know... maintainability?
So... I get the honor to take over this other program, which happens to have a critical and highly technical part of written in a fortran DLL. I had the distinction of porting it to C++ so we could overhaul the program. Now, look at the code at the top there. Imagine several thousand lines of code written just like this. Only in fortran. In a single function. With duplicate variables that never got deleted. Variable names that contain no vowels. Hacks to get around fixed-size arrays. Gotos like crazy. Conditions for loops that may or may not be satisfied properly. Break and continue statements that multiply like rabbits.
No imagine having to port this POS to C++, so that dynamic arrays can be used, having arrays accessing from zero instead of 1, and trying to rewrite proper for loops and if statements. And the original coder is considered a "stalwart consultant", and probably makes an order of magnitude more money than I do. And yes, there's another "mission critical" fortran DLL that I have convert over from the same coder.
Admin
I meant several hundred, not several thousand. My bad.
Admin
There is a single class in our source code that is just under 10,000 lines. I shit you not.
Admin
Whoever finds him first, please give him a good slapping on behalf of all namesakes.
Cheers,
Rik
Admin
I came across something similar while maintaining an old Oracle Forms 4.5 trigger - there was so much code in the one trigger that if you tried to add one more line of code it would not compile because it was too large! [:|]
Admin
++ Quoted for truth.[Y]
Admin
The truth is, some people were just never meant to be programmers. The "obvious and short" way isn't obvious to them. They usually grab some sample code from a related project and hack it into something that they think will do the task. When it fails, they have no idea why, and hack at it some more. When they stumple upon something that works, they still don't know why, so they can't clean it up --- they just leave it and hope it doesn't break.
Admin
I see that so often, it's frightening.
Then they come crying to me asking me what's wrong with it. I take one look at it and I'm like, "You didn't actually write that, did you?"
Admin
It... hurts! My brain is in pain! Somebody hit me repeatedly with a brick so I can sleep without seeing this!
Owwww!!!
Admin
Well put. I see this all the time when I step in as a lead on a failing project. Usually there is no code review and no unit tests used in the existing software process; it is more like plug-and-pray.
The guilty party is almost always the one who has been there the longest and is still in a junior position. Concentrating the team's efforts on this person's code usually turns the project around.
I doubt any of the examples submitted shown here would see the light of day if I was in charge.
Admin
Truer words are rarely spoken.
In fact, this is the first post on this site which has really made me go "WTF" out loud. More than any other sample so far, this one defies words or logic.
Admin
A routine part of my daily decompression is the scotch, whiskey, vodka, and/or brandy. I've had 3 of the 4 tonight, and I didn't even get around to writing a single line of code today.
At least on my current project (creating a new api to replace a scripted direct-database accesss process), I was able to convince everyone to drastically overhaul the data model. This involves being able to drop roughly 2/3s of the tables involved as being functionally redundant. [Un]fortunately, the person that first did the data model is no longer with the company. I suppose if they were, at least I'd have someone to throttle.
Admin
Oh, Lord, bad database schemas are even worse than bad code. They can easily cost ten times their original development cost to recover from.
Admin
Over here, we have a saying for people who can't think in a straight line: "Why make it simple if you can have it complicated?"
Admin
Just a small note: you'd probably want to make it a normal while loop (instead of a do-while), just to test whether the list is empty before removing an element. Of course, the code might never be called when the list is empty, or might rely on a side effect in RemoveEntry when the list is empty, but under minimal assumptions, I would prefer while (!IsListEmpty(List)) { ... }
Admin
I don't claim to know all languages in existence, but what kind of language has an uppercase BOOL keyword?
Admin
Actually the ASSERT isn't that stupid. I've seen BOOLs and bools that weren't bools, but a sort of Tri-state something. I've even seen if(statement){} executing the if if the statement returned false.
The rest of code well, wtf :-)
Admin
Does this code actually do what was "intended" (sorry about the quotes, but I am finding it difficult to tell exactly what WAS intended)
For this code to work (and bear with me, I am struggling to know what working means for this), the following assumptions have to be valid.
1) "entry_is_valid" must be a routine which inspects the item at the head of the list. having a reference to the list available via global vars (or possibly as an instance or class variable).
2) "RemoveEntry" must take the top entry and store a reference or pointer to this somewhere. and in the process dispose of any entry that is already stored in this reference/pointer
3) process_valid_entry and process_nonvalid_entry must access this reference/pointer as their argument
There is such a mix of ways of passing variables and handling items that this is less than clear
Naming conventions are not consistent, this clouds the issue. Specifically, I have assumed that "entry_is_valid" is a routine as it is updated nowhere, but the use of "routine()" calls makes me wonder whether the "enter_is_valid" is a variable that is updated somewhere else.
Before you can even think of fixing this code, you have to alter the routines and functions it calls to work on a consistent basis. This will assist the rest of the program by reducing reliance on globals and side-effects.
Having said all of the above, and taking into account some refactoring of the other routines, I would say that a neater way to implement this would be...
(dunno what language this code is in, so please forgive any problems with references/pointers etc)
while (!IsListEmpty(List))
{
item anItem;
anItem = removeEntry(List);
if entryIsValid(anItem)
{
processValidEntry(anItem);
}
else
{
processInvalidEntry(anItem);
}
destroy(anItem);
}
Please correct me if I am wrong, or there is a neater way...
Don't even know if the above code needs comments. maybe some to say WHAT it is doing and WHY it is doing it, but the HOW would seem to be self explanetory.
Admin
I used to work for a computer games company and one of the programmers there once wrote an entire front-end of a game in a single source file of 30,000 lines.
He knows who he is. 8)
Admin
That post above was meant to be in reply to the 10,000 lines of code in a single source file shocker.
Admin
I remember in VB having fun with the boolean True, False & Null. That was fun.
Admin
My recollection (which may or may not be correct) is that Visual C++ didn't get the intrinsic 'bool' type until VC++ 5 (I'm assuming that the code was written using VC++).
It wasn't until quite late in the lifetime of the C++ language that the concept of a true bool type (if you'll forgive the pun) was added - this was not exclusively a Microsoft problem.
I have distinct memories of using
in some of my early C code (20+ years ago)
Of course, premature senility may be kicking in and I could be talking bollocks.[8-|]
Admin
To Clarify a bit, the C++ language didn't get the intrinsic bool type to about the time of VC5 (It was proposal to the standards committee in 1995, and the Standard itself wasn't approved until 1998)
However, the most common uses of the "BOOL" type is in the Windows APIs which for legal reasons (yes, really, legal reasons), must be compatible with C code (which has never had an intrinsic bool type)
Admin
No, it is stupid. If you really wanted to check for that condition, then it should have been done explicitly, as :
ASSERT(entry_is_valid == TRUE || entry_is_valid == FALSE);
at the very top. "ASSERT(FALSE)" pretty much says that you don't know how to code -or- use ASSERT.
Admin
And you've turned an ASSERT(FALSE) into an ASSERT(TRUE), which would never actually do anything useful, regardless of where it was placed. The point of the assert was to guarantee the loop would break, one way or another. Its completely unnecessary, however. Some languages/compilers would likely warn on that code, because of unreachable code.
Of course, if that was really necessary, I'd prefer something like:
throw Exception("Stupid compiler. Shouldn't be here." );
[:P]
Admin
Part of the Microsoft XML Core Services - msxml2.h has 27782 (utterly unreadable) lines... It seems to be machine generated, which I guess is a bit of a wtf in itself!
Admin
Just to throw in some convoluted syntax...
do { BOOL is_valid = entry_is_valid; RemoveEntry(List); (is_valid ? process_valid_entry : process_notvalid_entry)(); } while (!IsListEmpty(List));And if anyone asks: Yes, this is valid C/C++ and will compile.
Admin
30,000 is pretty long. In Java you can compile a source file larger that ~65K so 10,000 lines tends to be about as large as you can make a class. The other thing was that this was no where near being a whole application. It was just a minor component. One of at least one-hundred 1000+ line source files.
Admin
No, because "BOOL", "TRUE" and "FALSE" aren't necessary the same as "bool", "true" and "false". BOOL is almost certainly typedef'd as an int, and therefore could easily have a value besides "TRUE" or "FALSE". My assert would break on exactly the same condition as the original assert(*), except that mine would a more meaningful error message, and even just in the code, documents the preconditions of the function.
(*) Actually, since the both the if() test the value manifestly, instead of against a particularly value, it is, in fact, completely impossible for the ASSERT(FALSE) ever to be reached, regardless of the values of entry_is_valid and discoveredEntry.
Admin
Now, my Java is a bit rusty. The language does require you to define the entire class in a single file, correct?
C# v1.0 does, but v2.0 won't. For C++, the declaration (.h file) must be in one file (barring kooky hacks), but the definition (.cpp files) can be split over as many files as needed.
Admin
JamesCurran: 'C code (which has never had an intrinsic bool type)'.
You almost correct. So I'll just nitpick that you're factually incorrect. C has the _Bool type.
Whether you have a compiler that actually compiles C is another matter, since what most people are worried about is C89, or waht used to be C. The current (and only standard) version of C was finalised in 1999, but it really doesn't seem to have taken off.
Admin
Yes, that is correct. Every class must be in a single file. You can only have one public class per file. Generally people just put all top level classes in their own file even when they are not public for clarity. The compiler will choke if the source file contains more than 65536 bytes, if my memory serves me correctly.
Admin
Actaully, MOST windows variables have upper case, and I DONT think it's legal reasons, but to ensure consistency between windows versions, especially when the switches are/were made from 16->32->64.
Forexample you have ULONGLONG which is really : unsigned __int64.
You have WORD which is unsigned short, and DWORD, which is unsigned long. BYTE is unsigned char. INT_PTR on the other hand can be __int64 for windows64 and int for windows32. LPCTSTR is a constant char pointer, which can be either unicode or ascii depending on the build of windows installed. There's about a hundred of these kinds of uppercase variables.
Go to your MSDN library and look up "Windows Data Types" or "Windows API".
I personally tend to use instrinsic types like bool instead of BOOL and the windows API functions don't seem to mind. The exceptions I make are with exotic placeholders like POSITION and COLORREF, to avoid casting problems.
Admin
I'm not really a C guy, so please correct me if I'm wrong: Besides the totally flawed code, wouldn't a "continue" have the same effect as that goto directly to the end of the loop??
Admin
If having you own -ism isn't just quite good enough for you, you could shoot for the status of the programmer from whom I've inherited much code... There still is the occasional use of -ism's though he has been gone for 6 months. (Actually, there is a set of -ism's) He remains known as 'ED' aka Evil Derek.
Admin
Alright, make with the WTFs.
Admin
Other than bypassing a second call to "IsListEmpty()", yes they would be the same....
Admin
Ok, one of my favorites is an implied foriegn key relationship from a transaction table to a cardholder table, sometimes... sometimes the foreign key column value corresponds to an account table. kinda just depends on how the code feels at the time, but the best I could figure was it worked like this.
First a cardholder record is created, if the user is new the new cardholderID is used as the SourceID for the tranaction table. If it is determined there is an existing record for that person in the account table, the AccountID is used for the SourceID in the transaction record. (cool, huh?)
It certain situations, a new transaction must be created for for that person based on the previous transaction. It then uses that person's AccountID as the SourceID for the new transaction. Then another process uses that new transaction (picks up all new transactions) to collect fees. Well, that fee collection process assumes the SourceID references a CardHolderID.
Anyone see what's coming here? Yes, sometimes is does not find a person to collect from essentially giving a person free product. But even better than that is that it sometimes find a match in the cardholder table to the value which is supposed to be an accountID and ends up billing the wrong person.