We pay for user submitted tutorials and articles that we publish. Anyone can send in a contributionLearn More
I have been wanting to write this series of articles for a long time. Over the years I have come across very bad code examples and I have always wanted to share them with you guys, kind of in the way of “Watch and DON’T learn” .
Everything you are about to see in this series is based on my personal experience and completely real, so before we begin I have only one request:
Don’t try this at home!
Ok, lets get started.
First, lets take a look at the code. The code is of a real function I found, that determines the color of an object (All variables were changed to keep confidentiality), brace your selves:
How long was that! Lets start butchering it from top to bottom
I don’t know about you, but I think that 10 method variables are just too much!
Once you are sending that many variables to a method, you are either writing a method that does too much, or you are overlooking the possibility that some of these variables have similar uses and responsibilities, and should be grouped into a structure or a class. That object will be passed to methods instead of passing so many variables.
This is a very good example of coding with no prior design, every time there is a need to add some functionality the result is:
“Sure, lets add to method x another variable, some “if elses”, a “switch case” and that’s it! It works!”
How can you write a 168 lines long method?
After 20 lines you have no recollection of: what the method name is, what it is supposed to do, Why are you reading this method, and what got you in this method in the first place.
Debugging and understanding this code is just great this way…
No doubt, This code had to be broken to at least 10 methods and its a no brainer. Switch Case and If Else expressions makes breaking pieces of code into smaller chunks so much easier.
Try Catch phrases have a very important role in coding. Using them correctly is an art known to not many people. On the other hand, abusing them is easy as hell. Sure, Why not wrap the whole code in a big try catch and then if there’s an error, we will have no way of knowing where it happened. What to do? Just throw out a message box that says
“Error in Coloring”
That will help the user know what is wrong… NOT!
And of course, Lets make that message box Critical, so the user will get nervous… (line 166)
Throwing a new exception? Or letting it bubble up? Nope, why bother….
I must say, and I think I speak for 99.9% of the programmers:
I hate commenting my code!
But, I do it anyway cause it is so important, not only for the next guy who is going to handle my code, but also for me one month later, when I have to fix some bugs there, and won’t have a clue as to why I wrote this line or another.
Here, I have counted 10 comment lines on 168 code lines, and I can tell you that the comments written weren’t all that informative…
I know this is more of a flavor question, but using 5 If Else expressions one inside the other? And some Switch Cases for desert? I don’t like it.
that is a recipe for a major Bug. Everyone knows that sometimes you have no choice, but I can tell you that here, there was a choice…
There is no doubt that writing this code will someday come back at you and bite you in the ass, your only hope is that you might not work there anymore…
Well that is all I had to say about this code. If you see anything else that I did not address, or disagree with something, please do comment.
Don’t forget to Grab our Feed so you get the next post in the Terrible coding examples series.
And as I said earlier:
Don’t try this at home…
Tags :AbuseBad CodeBad ProgrammingCodingErrorIf ElseprogrammingTry Catch
Copyright © 2012 Dev102.com
Breeze : Designed by Amit Raz and Nitzan Kupererd