- 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
hey, at least they're getting there money's worth... if you consider the $ / line of code
Admin
i think it is not a bad code
Admin
This annoys me: "a blank date is considered valid as it is not a mandatory field". The absence of a date isn't a valid date, whether or not some specific format regards the date as optional. When the data isn't there, don't try to validate it, at least in a place like this.
Admin
Unfortunately this doesn't take account of the fact that years divisible exactly by 100 aren't leap years unless they are also divisible exactly by 400.
Admin
Happily accepting months in the range -9 to +99. In fact, I suspect it'd also accept garbage characters in there as I think Cint would just return 0 for the month. But then, should we REALLY complain? Plenty of invalid years will also be accepted.
Admin
IsDate() anyone?
Admin
I personally like this:
It doesn't get used anywhere, but it raises a question: does it make date invalid if it happends to be in the future?
Admin
Too much consultant hate. Don't hate the player, hate the game.
This awful code is not awful because it was done by a consultant, it is awful because it was done by someone who didn't realize that an IsDate function exists.
Admin
Sure, it doesn't exists, also the dates in the past aren't here anymore, so the must be invalid, so, the only date allowed need to be this exactly moment, but, anyway, a nanosecond after, it gets invalid.
Hard time to the user to type the date at super high speed, and with a presission of a atomic clock.
Admin
I think this guy is missing the obvious optimization -- the variable "validDate" should be "valiDate." The reduced size of the code will make it compile and run faster.
Admin
Nope. This code would not be awful if it simply were a copy of the logic of IsDate().
It is lousy code. Others have mentioned flaws. My turn: a for loop index is modified inside the loop.
Sincerely,
Gene Wirchenko
Admin
I didn't make a complete count. But at first sight, I'd say that this is truly value for money - all this code for a lousy 2 grand, that's well under $100 per WTF!
Admin
The real WTF here is that the month and day are switched.
Admin
Shouldn't that be "For i = 3 To 6 Step 3" in the delimiter check loop, lest it insist that characters 3, 4, 5, and 6 all be delimiters?
Admin
This is AMAZING code. I just can't go into why at the moment.
Admin
<gag>Note that the loop index is modified inside of the loop.</gag>
Sincerely,
Gene Wirchenko
Admin
The biggest reason why this code sucks is that it's not reusable between regions that format dates differently. IsDate() will work no matter how the date is formatted, but this piece of crap won't.
Admin
I may be rusty in VB (that is what this is yes?)... but I seem to remember "stop" causing the application to exit. If "stop" is in the middle of this function, will it simply exit the application once that statement is reached?
Admin
The next time someone bitches at me for using
try
{
DateTime.Parse(input);
return true;
}
catch(FormatException)
{
return false;
}
I'll just show them this example.
And what the heck does Stop do? Is this a telegram?
Admin
Is this VB?
And if so, wouldn't the "Stop" statement invoke the debugger before the bad loop?
Admin
It is probably LotusScript which is Basic based.
In which case a "Stop" statement is a debugger command to pause execution when the statement is reached.
Admin
If my VB memory is correct the stop statement stops execution immediatly, no falling into the debugger, no graceful exit. This can't be production code....
Admin
Only in America.
Admin
That was supposed to be:
Only in America.
Admin
Yeah, but it's extra awful because a guy got paid $250/h for writing it. I'm a consultant myself and it pisses me off because my company takes half that much, yet none of us would write crap like that.
What you're looking at is the reason why IBM is the world's 3rd most valuable brand. It has an estimated value of over $50bn (the brand alone, not the company) because it allows you to charge $250/h for incompetent coders. Just like the world's most valuable brand allows you to charge three times the normal price for caffeinated, excessivly sparkly sugar water.
Admin
My guess: this check was later removed because it is senseless, only the variable currentYear remained.
IMO, this code is not a big WTF. It contains holes, but that doesn't make it a WTF. It probably reinvents the wheel, but on the other hand, it is not a duplicate of IsDate. IsDate allows for a range of possible date formats, but probably that was not what the customer wanted.
250 USD/h is a WTF on its own, unless the other code written by that guy contains special knowledge that makes it worth the money or the guy produces at least 2500 reasonably well written lines of code per day.
Admin
Yeah, "-6/31/WTF?" should be considered a completely valid date. Make sure the user is using separators I find acceptable --that's the most important thing. And for God's sake, don't accept something like "11/2/2005" or "Nov 2 2005" or any of that nonsense. I'm a $2500/day consultant and you shall conform to my format for dates. Resistance is futile.
--Rank
Admin
Eh? How can a function that claims to check if a date is valid, yet accepts dates such as "27/14/2005", "25/ab/cdef", or "29/02/1900" NOT be a WTF?
Admin
<font size="4">Well -- way back when I started programming, the IBM standard was 11 lines of debugged code per programmer day.
Lessee here. Not counting comments and blank lines, I make it 83 lines of code, just a little more than 7.5 days' worth.
7 and a half days for $2000. . . roughly 267 bucks a day.
Bargain!</font>
Admin
This code doesn't need a debugger, it needs a debuggerer, because it's seriously buggered.
Admin
It's buggy, some checks are missing (especially the 27/14/2005 case).
Are you sure about the "25/ab/cdef" example?
"29/02/1900" is probably a non-issue in the context of the business requirements.
Is every piece of buggy code a WTF?
Admin
I haven't use Basic during the twenty last years but what is this "Stop" at the middle?
Is it when you have to pay? :S
Admin
It's awful because this slimy hairball was hacked up by someone who got paid $250/hour for writing it, for the client's sake I hope that was just a cut'n'paste from the 'consulants' old code and not done on the billed time.
Admin
No. I guess it depends on what this language actually is, and what it does when an attempt is made to evaluate CInt("ab").
I just tested it in VBA. Execution is halted and a runtime error is raised. Obviously not what you want to happen in a function that is intended to weed the bad input from the good.
According to comments earlier in the thread, there are languages that evauate CInt("ab") as 0. That would pass the rest of the check. Also not what you want.
Since no check is made to the results of the CInt (except for the day number), there is no way that "ab" in month or year would ever result in this function returning "False" - either a run-time error, or "True".
No. But if it's so buggy that even the simplest module test would have failed, it is.
If it uses a for loop without step but increases the counter inside the loop, AND produces buggy results, it is. (It would even be a WTF if it did return the right results).
If it meticulously (sp?) checks the day number, yet completely forgets to check the month number as well, it is.
Need I go on?
Admin
Yah, it's buggy code, but it's not a WTF. A WTF is something that takes your breath away makes you spit out whatever was in your mouth and go "dear goodness how could this travesty exist!" This is just somebody who re-invented a not particularly interesting wheel, and didn't do a great job at it.
If I see code like this in the apps I work on, I just shake my head and think "well, hopefully I'm getting paid more than this guy" (IBM made 2000$/day, but probably gave this guy peanuts).
When I see a WTF it is a thing of perverse beauty that requires I either take a 15 minute break to stop being so angry with the person who wrote it, or immedately send the code snip to a friend, or a site like this.
Admin
At $250 an hour this should be worth nearly $10 for the time to check if a function exists and write code that works. I hate it when it's assumed the guy can code just because they charge $250/hour when in reality they're just ripping everyone off and giving the rest of us a bad name.
Now I can't get a job because I charge like $50 an hour for coding and people just assume its 5 times worse than this crap.
Morons must be eliminated.
Admin
One of the (many) WTFs here is that there is a for loop at all. It's not like it's a performance-sensitive piece of code, but a loop to go through *two* values? *Two*?
Admin
I think someone already mentioned this but in VB6 (I think the code is VB6 but i may be wrong) a Stop statement will halt execution and display a message box saying "Stop statement encountered" and the program will just terminate following the user clicking OK, which is really really bad.
If you wanted to halt execution and bring up the IDE, you'd do something like debug.assert false which won't trip up execution if running a standalone exe.
Admin
The non-use of IsDate() is partly forgivable - the situation may have some special requirements, but the function should really just be a wrapper for IsDate() that checks the special requirements first. Beyond that, I challenged myself to find as many WTFs as possible (items in italics are things I didn't see mentioned yet):
- currentYear calculated but never used
- Validity of zero-length strings should be an optional parameter
- Format should be an optional parameter
- Leading zeroes should be optional
- Use of >< (unusual, even if valid)
- Non-use of For/To/Step
- theYear and theDay can never be blank
- Input to Cint() not validated
- "9 4 6 11" in comment
- Invalid leap year logic (albeit valid for 1901-2099)
- "if dayNumber > 0 and dayNumber < daysInMonth" should be refactored out of the Select/Case
Damn, I missed that it would accept invalid months and years. And that a For loop is silly when you're only going to have two iterations.Admin
VB6 uses <> as the check for not-equal. There is no !=
Admin
Interesting. I remember that when using QBASIC (hey, I was only a kid at the time!), putting an alphanumeric string into the CINT function was supposed to yield the answer zero, according to the language specifications.
Admin
Admin
In the early- to mid-80's, the standard was anywhere from $8 to $20 for each line of debugged code.
Of course, that was before languages like C made you write ten lines to accomplish the simplest of tasks, and before languages like C# only told you in the back of the manual what the compiler would do with your code.
Admin
Even more unfortunately, it doesn't matter. Assuming that uninitialized variables are set to 0 in this language, every year is a leap year.
Admin
Totally undefensible code. If you do not have the luxury of upgrading to .NET 2.0, use ParseExact and supply the expected types of formats, because if you don't there will be very interesting results:
Like DateTime.Parse("10,1") which on the surface of things is not the 10th of January 2005.
Admin
here here!!
Oh wait, I work for IBM.
Ignorance is bliss, ask any of my blissful colleagues.
Admin
If this data is coming from the UI, rather than from a file, then I would strongly recommend having separate entry fields for the day, month and year - or use a DateTimePicker, which won't allow an invalid date to be selected. If it is coming from a file, then I agree that ParseExact is the right solution. .NET 2.0 even has a DateTime.TryParseExact method which simply returns False if the parse fails, rather than throwing an exception.
Using the separate fields doesn't eliminate the need for validation but would get rid of a lot of the horrible parsing in this code. A simple way to validate would be to construct a DateTime instance using the constructor which takes a year, month and day and catch the ArgumentOutOfRangeException that will occur if it's not valid.
On the leap year issue - assuming you need to handle 29 February 2000, then the code does need to be corrected. Otherwise the code will be fine for 395 years so it may be a little premature to suggest that the bug be corrected. Having said that there are enough bugs here already, and the fact that there are simple substitutions available, that I'd rewrite it, on the basis of the research result that bugs tend to come in clusters: if there are a lot of bugs in one area of some code, there are likely to be even more lurking.
As for the $50M worth of donated code, that was presumably written by full-time software developers rather than consultants and so we can't write it off completely... :)
Admin
Admin
Look at these lines:
If Len ( dateString ) >< 10 Then
validDate = False
Exit Function
End If
<FONT color=#000000>theYear = Mid ( dateString, 7, 4 )
</FONT><FONT color=#000000></FONT>
<FONT color=#000000>If theYear = "" Then
validDate = False
Exit Function
End If
</FONT>
When theYear is not ""?
Admin
"27/14/2005", "25/ab/cdef" are definately invalid, but i wouldn't call "29/02/1900" invalid. :) Allthough it might be some time ago, 29th of Feburary 1900 is a pretty valid date. :)
(unless you're from usa ofcause. Their date-format has been scrambled in some arbitrary order).
<FONT color=#ff0000>Something didn't quite work out ...
- CAPTCHA Validation Incorrect </FONT>