Comment On Your $2000 Function, Rebilled

hey, at least they're getting there money's worth... if you consider the $ / line of code

i think it is not a bad code

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.

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.

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.

IsDate() anyone?

I personally like this:

'calculate the current year to check the date is not in the future
currentYear = Year ( Now ( ) )

It doesn't get used anywhere, but it raises a question: does it make date invalid if it happends to be in the future?

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.

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.

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.

Anonymous:

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.

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

Alex Papadimoulis:

I've often wondered what kind of code you get when you pay a consultant $250 an hour.

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!

The real WTF here is that the month and day are switched.

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?

This is AMAZING code. I just can't go into why at the moment.

Anonymous:

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?

<gag>Note that the loop index is modified inside of the loop.</gag>



Sincerely,



Gene Wirchenko

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.

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?

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?

Is this VB?

And if so, wouldn't the "Stop" statement invoke the debugger before the bad loop?

 

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.

rbriem:

Is this VB?

And if so, wouldn't the "Stop" statement invoke the debugger before the bad loop?

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

Only in America.

That was supposed to be:

Anonymous:

The real WTF here is that the month and day are switched.

Only in America.

Anonymous:

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.

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.

Magic Duck:

I personally like this:

'calculate the current year to check the date is not in the future
currentYear = Year ( Now ( ) )

It doesn't get used anywhere, but it raises a question: does it make date invalid if it happends to be in the future?

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.

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

ammoQ:

IMO, this code is not a big WTF.

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?

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!

rbriem:

Is this VB?

And if so, wouldn't the "Stop" statement invoke the debugger before the bad loop?

 

This code doesn't need a debugger, it needs a debuggerer, because it's seriously buggered.

Hugo:

ammoQ:

IMO, this code is not a big WTF.

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?

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?

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

Anonymous:

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.

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.

ammoQ:

Are you sure about the "25/ab/cdef" example?

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

ammoQ:

Is every piece of buggy code a WTF?

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?

Hugo:

ammoQ:

IMO, this code is not a big WTF.

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?

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.

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.

Gene Wirchenko:

 My turn: a for loop index is modified inside the loop.

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

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.

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):

1. currentYear calculated but never used
2. Validity of zero-length strings should be an optional parameter
3. Format should be an optional parameter
4. Leading zeroes should be optional
5. Use of >< (unusual, even if valid)
6. Non-use of For/To/Step
7. theYear and theDay can never be blank
8. Input to Cint() not validated
9. "9 4 6 11" in comment
10. Invalid leap year logic (albeit valid for 1901-2099)
11. "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.

emurphy:

Use of >< (unusual, even if valid)

VB6 uses <> as the check for not-equal. There is no !=

Hugo:

ammoQ:

Are you sure about the "25/ab/cdef" example?

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.

*snip*

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.

Ytram:

The next time someone bitches at me for using

try
{
    DateTime.Parse(input);
    return true;
}
catch(FormatException)
{
    return false;
}

I think the best solution is to move on to .NET 2.0 and use DateTime.TryParse. Until then, your code is entirely defensible: just point out localisation.

Anonymous:

Well -- way back when I started programming, the IBM standard was 11 lines of debugged code per programmer day.

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.

Anonymous:

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.

Even more unfortunately, it doesn't matter.  Assuming that
uninitialized variables are set to 0 in this language, every year is a
leap year.

Ytram:

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?

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.

brazzy:

Anonymous:

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.

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.

here here!!

Oh wait, I work for IBM.

Ignorance is bliss, ask any of my blissful colleagues.

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

12. Check for whether date is in future only checks value of year
13. In addition to #6, Increments for loop counter by 2 instead of 3 (6-3=2?)
14. YearNumber is never assigned a value
15. Month and year are not checked for bounds
16. Function is called as Integer but assigned Boolean value

Look at these lines:

If Len ( dateString ) >< 10 Then
            validDate = False
            Exit Function
End If

theYear = Mid ( dateString, 7, 4 )

If theYear = "" Then
            validDate = False
            Exit Function
End If

When theYear is not ""?

Hugo:

ammoQ:

IMO, this code is not a big WTF.

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?

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

Something didn't quite work out ...
- CAPTCHA Validation Incorrect