<cfsilence>

Daily WTF - Check Out This Gem

Posted At : October 9, 2008 5:09 PM | Posted By : todd sharp
Related Categories: ColdFusion

A friend was searching through some legacy code when he found this absolute beauty (names changed to protect the guilty):

Wow where do I begin explaining whats wrong with this code? Actually I think it speaks for itself.

Comments (16) |

Print |

Send |

Code |

del.icio.us |

Linking Blogs | 2186 Views

Comments

[Add Comment]

Haha that's some rich stuff. I guess it's just like a total misunderstanding of the whole try/catch thing. And the error type thing. And the thing about why the heck a product ID can't be over 10 - a magic number scenario.

So nice.

# Posted By Nathan Strutz | 10/9/08 7:28 PM

This is just classic material, where do you find this stuff!

# Posted By Dan Vega | 10/9/08 11:41 PM

I still think you should explain what is wrong so people can learn, because someone will find this in google and cut and paste the example into their code.

# Posted By Dale Fraser | 10/10/08 1:12 AM

Clearly this person is trying to bring things from another programming language, which he/she probably did wrong there too.

# Posted By Gary Gilbert | 10/10/08 5:14 AM

Well Dale, Nathan has already started, but let me take a shot.

First of all, I have a problem with the fact that the code checks the len of the productID and throws a custom exception if it is not numeric or the length is greater then 10. Since when is an number not a number if it is greater then 10 digits?

Secondly, the whole try catch block is a giant waste. I imagine they probably wanted to put the cfif statement inside of the try/catch block - but even so the cfcatch isn't doing anything. No logging, no email, no nothing!! Even if the code were written correctly, I find it to be a bad practice to throw a custom exception and immediately catch it within a try/catch block...what's the point? I would venture to guess what they really were trying to do is simply validate the productID - if so then I'd get rid of the throw/try/catch and just validate per your business rules and create a friendly error message to show the user if the validation fails.

# Posted By todd sharp | 10/10/08 9:44 AM

HA!! That is nothing. The guy before me was a walking talking WTF.

Check this out.

This:
http://mgt.pastebin.com/f7fb1e536

Renders this
http://www.myinternetisbroken.com/files/Shite_OutP...

# Posted By Gerald Guido | 10/10/08 10:11 AM

You have got to be kidding me!!!!

What a load of crap that is!!

# Posted By todd sharp | 10/10/08 10:20 AM

@gerald

actually that code isn't that bad at all. pretty readable from the code itself and it's commented nicely.

the thing is, when was this code written? if it was written back in the 4.0 days, you didn't have a lot of the functionality that we do today. functions, QoQ and a lot of things didn't exists, but arrays and structures did.

I'm only stating this because before your laugh and comment on what someone wrote, you need to know the whole story.

So if this was written back in the 4.0 days, put this guy on the back. If it was written 2 weeks ago under Cf8, kick his ass.

# Posted By dfguy | 10/10/08 10:55 AM

@Gerald,

Wow, I don't think in all my years I've ever seen such a fundamental lack of understanding the use of a query. At first I assumed that this was stateless, with no DB in the back or something ... until I saw the CFQUERY and the third set of CFSET tags. Ouch

-jfish

# Posted By Jason Fisher | 10/10/08 10:59 AM

@dfguy

This was done just before I came on... V.7 so it was a couple of years ago. This guy was a piece of work....

I ranted about here.

http://www.myinternetisbroken.com/index.cfm/2007/4...

Thank good I don't do all that maintenance stuff but when I do, it makes Baby Jesus cry.

# Posted By Gerald Guido | 10/10/08 11:15 AM

@gerald

after reading your post and comment i must recant my previous position.

IT YOUR DUTY TO FIND THIS PERSON AND DESTROY HIM!!!

I only wish you can give us his name so I may warn my HR department about ever hiring him.

# Posted By dfguy | 10/10/08 11:34 AM

Im assuming the code posted is not complete.

Surely there would have at least been something in the catch section, not much point going to the trouble to throw and catch a custom exception and then do nothing.

# Posted By Dale Fraser | 10/11/08 2:06 AM

The code is complete - I only wish I could have thought to make this up!

Not only that, but since nothing is in the try block nothing is going to be caught.

# Posted By todd sharp | 10/11/08 2:23 AM

Would you mind if I used these as examples of how *not* to code, in an internal seminar I have coming up? I will, of course, give you credit.

Kind regards

</cliff>

# Posted By Cliff Pearson | 10/13/08 11:56 AM

np on this end Cliff. Do me a favor and NOT give me credit ;-)

# Posted By Gerald Guido | 10/13/08 12:48 PM

@Cliff: I don't mind - and no need for credit really.

Might want to also keep an eye on dan vega's blog I think he's going to start posting some similar examples.

# Posted By todd sharp | 10/13/08 1:53 PM

[Add Comment]