I'm just wondering what are some of the classic mistakes we all make. I think before working on writing good code, you must learn to recognize bad code... especially when it's YOUR code!
It's gotta be the Big Ball of Mud.
A Big Ball of Mud is a haphazardly structured, sprawling, sloppy, duct-tape-and-baling-wire, spaghetti-code jungle. [...]
[...] this pattern is most prevalent because it works — at least at the moment it is developed. However, programs of this pattern become maintenance nightmares.
http://sourcemaking.com/antipatterns
Spaghetti Code and Fire Drill seem to be pretty common to me.
You can find a decent collection of these here: http://forums.thedailywtf.com/forums/p/8599/163647.aspx
I find premature generalization to be the worst one:
Creating complexity in your software, because you might need this later on.
I love seeing the "For Loop Case Construct".
for(int i = 0; i < MY_NUM; i++)
{
switch(i)
{
case 1 :
<snip>
case 2 :
<snip>
.....
}
}
Good Times!
my favorites
- hardcoded magic-number crap -> int myNumber = 23,333;
- copy and paste code -> aaargh, spaghetti @ its finest
- double-checked locking -> wrong usage of locking (volatile ftw)
- lava flow -> sourcecode which is no longer needed
- blob -> god object
I've seen a few very easy to understand ones over and over again:
- Magic numbers. Even worse is when magic numbers are passed through multiple tiers.
- Excessive use of "I'll get to it later." I've worked with several developers who put so much TODO, stubs, and hard coded return values in their code that they forget what they haven't implemented yet. It's pretty sad when a stub ends up in production and it comes back to haunt you a year later.
- Methods which rely on other methods to be called first or they won't work correctly.
Something I see a lot here that shoots developers in the foot more often than not: premature optimization.
if (someBool == true)
...instead of...
if (someBool)
Or running .ToString()
on a string
. I've been guilty of both.
In a non-memory managed language I see the lack of try / finally blocks when objects are created a lot.
Or in Delphi where they assign an object an owner, and then free the object quickly anyway, which results in two notifications to the owner that are unnecessary.
Also the use of a try / catch (or except) when they should have used a try / finally (or vice versa). I know some programmers who do this religiously because they don't like the other.
Singleton + public static access!
"We'll only need one of these so lets make it a singleton and statically access it!"
Yea, thanks i'm now trying to test the code and my unit test is now an integration test thanks to you.
Pattern-o-mania
"We need to make some software.... how do we do that? I know! Lets use all the patterns in this book, even if we don't actually need them!"
Cannot... understand... insane and crazy architecture.... what's that... another factory?.... arrrrggghhhh!
Two roads to the same place
"This is a completely different operation, so lets make different methods and different code paths with the powers of copy+paste+little edits"
Thanks, now I have to fix bugs in more than one place.
Form first development
"I like Visual Studio it helps me make applications without knowing what I am doing"
Must....resist....urge...to...track.. down.. former.. developer.. with.. knife....
Passing on the problem
"Hmmmm, how do I handle this problem. I know i'll add another method and let the caller decide!"
Okay so your wrapper exposes more methods than what you are wrapping. Thanks, that really simplifies things.
Object of doom
"Wait.... why does anyone need to create their own application, they can just use my object to do EVERYTHING"
Even if I spend a week reading the documentation I still wont know what all your method calls do. I just want to do xyz! :'(
Reflection Abuse
"Now i'll just use reflection to invoke this method and hardcode the name of the method into the source..."
Thanks for making some invisible breaking changes. If only they hardcoded their personal contact details into the source as well so I could hunt them down......
XML for everyone!
"Hmm.. its not working... mebbe I just need some more xml"
No, you need more cyanide.
A complete disregard for error conditions
if(someObj == null)
{
DoMyFunkyThang();
}
else
{
// ah fooey and gosh-darn it, this will never happen
}
IMyInterface someInterface = someObject as IMyInterface;
someInterface.PerformSomeOperationWithoutEvenWorryingThatItMightBeNull();
Huh? NullReferenceException.... this is nothing to do with me yet I have to pull out the debugger and solve this to perform todays task. No clues about what is wrong.... :(
Using exceptions in NORMAL PROGRAM OPERATION
I have no idea what these people are thinking
Even WCF does this..... why MS? You did so well at this in .NET 2.00!
Storing XML in a database BLOB field.
Bonus points if the xml looks something like:
<record>
<field name="first">asdF</field>
<field name="last">jkl</field>
</record>
Double-bonus if the XML was generated using a homegrown engine that doesn't always generate valid XML.
try
{
DoSomeImportantThing();
}
catch( Exception )
{
}
Not fun.
Reinventing the wheel:
Hey, I need to sort, this list. I better implement a sorting-algorithm. The one in the library is surely not as good as mine.
Probably Yet another meeting will solve it.
You have a problem with schedule. To solve this problem, you start making meetings. People that should be involved with delivering the project start to be involved in this meetings. Project gets later, oh... Yet another meeting will solve it.
Primitive Obsession (see here here and here)
Fear of defining task-appropriate classes/types, which leads to representing everything in globs of primitive types (int, char, boolean, ...). Often driven by excessive concern with "efficiency" (i.e. execution speed at the expense of clarity, maintainability, and robustness).
- Premature optimization
- Copy'n'Paste programming (copying instead of parameterizing/generalizing)
I'm not a believer in lists - they just get outdated too soon. Instead, I'd refer you to the original Wiki - http://c2.com/cgi/wiki? and the page there - http://c2.com/cgi/wiki?AntiPattern
A warning though - you can read that site for days, though you'll likely be a better developer for it.
Using instance level variables when local variables would work just fine. In a multi-threaded environment this can cause some nasty problems.
Example:
SomeClass{
private Object rtnValue;
public Object someMethod() {
rtnValue = lookupObject();
return rtnValue;
}
}
Putting implementation into the header file ... makes debugging impossible!
I particularly like when when people don't change the name of the automatically generated Form1. This usually indicates that there is not likely to be any attempt at separating UI from logic, and that I'm about to spend the next six hours spelunking through the vast spaghetti maze to find the ten places I need to change in order to make a minor adjustment.
Code files containing many methods with no documentation at all with bodies that all look like this are fun too.
public Class1
{
public void Temp()
{
//HACK: Fix this later maybe
throw new NotImplementedException();
}
Added Bonus: Under Visual Studio 2005, if method stubs such as this are auto-generated the exception message was specified as a constructor parameter breaking CLS compliance for nothing because the specified message was the same one as the default constructor uses.
I'd love a name for this (probably a variant of "reinventing the wheel"):
Where I work we effectively have lots of similar projects. The goals are very different, but the technology is similar.
But when a project implements new functionality that may be useful in other projects, they often don't implement it in a reusable way. Either because they don't think about it, or because it takes longer.
So projects ends up implementing the same thing again and again (to save time), but in slightly different ways.
"Magic Button".
I remember the days when I used to write VB code and all DB connection, UI data population work used to be behind one single button click. :)
There is an (old) article called "Resign Patterns", which can be found on various web sites (here for example).
While it is meant as a fun article, I think that there is still quite a lot of truth inside. Some of these "resign patterns" can be found in existing/productive code bases.
One of the most used anti-patterns are Global State and Singleton.
How to solve it
You may want to check The Clean Code Talks for some advice on how to correct them (usually with Dependency Injection and Inversion of Control).
A great article that introduces you to Dependency Injection is: Inversion of Control Containers and the Dependency Injection pattern by Martin Fowler.