views:

2521

answers:

32

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!

A: 

I don't make mistakes! </sarcasm>

Geoffrey Chetwood
StackOverflow: Where any hint of humor is downvoted.
Geoffrey Chetwood
Humor is OK, but this doesn't answer the question at all, so of course it's going to be downvoted.
Outlaw Programmer
Leaving it at zero works too.
Geoffrey Chetwood
Make snarky useless replies using the comment feature, where it can't be downvoted :).
17 of 26
I thought I was wrong once, but I was mistaken ;-)
travis
Fair enough. Guess I'll start saving my downvotes for wrong or misleading responses.
Outlaw Programmer
@Outlaw: That would be my recommendation, I am not trying to whine, but it seems silly and petty to downvote for any kind of humor... I don't want to be a part of a site that looks down upon a little humor now and then...
Geoffrey Chetwood
Unfair downvotes are ususally countered with an upvote - which results in a net rep. gain! Eg. I just voted you up cos I don't think your comment deserves a -2. IMO, 0 is for relevent but not yet noticed stuff, -1 for irrelevant but harmless, and lower is for stupid/wrong stuff.
Blorgbeard
I thought the comments were where the sarcasm went.
Dan Blair
@blorgbeard: Interesting. I would say common sense would dictate 0 would be a useless comment that causes no harm.
Geoffrey Chetwood
I was just about to say that you were downvoted for incorrectly not using an opening sarcasm tag, but then I was like... OH SHI-- XD
Sandor Davidhazi
@Geoffrey Chetwood my reasoning is that a new answer coming in might be useful, and should therefore have a higher score than a known-useless answer.
Blorgbeard
-1 for invalid XML.
kbok
+16  A: 

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.

Shog9
+19  A: 

http://sourcemaking.com/antipatterns

Spaghetti Code and Fire Drill seem to be pretty common to me.

Leahn Novash
This web site is nice, but it is blatantly ripping off a book called "Antipatterns"
Ken Liu
duh. it's their book!
+2  A: 

You can find a decent collection of these here: http://forums.thedailywtf.com/forums/p/8599/163647.aspx

Geoffrey Chetwood
+31  A: 

I find premature generalization to be the worst one:

Creating complexity in your software, because you might need this later on.

Daren Thomas
Ugh yeah. Otherwise known as the YAGNI principle.http://en.wikipedia.org/wiki/You_Ain't_Gonna_Need_It
Blorgbeard
On the flip side, there is the problem of code designed with so little thought to the future that it must be rewritten to have new features added. I've always said "plan for the future, program for today". But that is open to abuse.
Torlack
I don't know, I think code that has to be rewritten to have new features added is just badly designed in general. There's no need to specifically design for the future - a good design is inherently extensible, IMO.
Blorgbeard
also known as bureaucracy.
RamyenHead
+12  A: 

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!

Dan Blair
If only there were some other way to get those cases to execute in the proper sequence. :)
Bill the Lizard
.... .... lol (took me a while to work it out though, doh)
Sam Hasler
Comedy gold, Jerry
Meff
Like rollercoaster transportation - unnecessary loops.
micahwittman
+3  A: 

Singletonitis: That got to be the most used pattern in Java.

Shadow_x99
+6  A: 

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
DeeCee
+1  A: 

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.
Daniel Auger
+3  A: 

Premature optimisations.

ionut bizau
+1  A: 

Something I see a lot here that shoots developers in the foot more often than not: premature optimization.

17 of 26
+32  A: 

Commenting out code instead of deleting it.

Blorgbeard
Fully agree - thats what your source code control tools are there for
RichH
Absolutely disagree. Small sections of commented out code (with comments) as an example of a how <i>not</i> to change a particular item, or how a previous logical-looking implementation was subtly broken, can prevent a lot of wasted time down the road as the reasons are repeatedly re-discovered.
Zed
I'm not saying never put example code in comments - I'm talking about this kind of crap: "//LogVerbosity = 0; // removed by bb, 2008-10-15, now done in FooInit function
Blorgbeard
Wow, yours are marked with dates? And recent dates at that? Ours are commented // removed by $devWhoLeftIn2003
Adam Jaskiewicz
+4  A: 
if (someBool == true)

...instead of...

if (someBool)

Or running .ToString() on a string. I've been guilty of both.

travis
remember some languages are not strongly typed, so it's not so bad in that case. But then I assume you mean in the case of a strongly typed language from the name 'someBOOL'...
Frep D-Oronge
The someBool == true is actually recommended in Code Complete, as it is more readable out loud, apparently.
harriyott
You nailed my two biggest pet-peeves. Upvote for you.
Kevin
+1  A: 

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.

Jim McKeeth
+57  A: 

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!

Quibblesome
Quarrelsome,I do NOT care if YOU are not PG-13. What I do care about is avoiding such a good answer to be marked offensive because of that single word.
Jon Limjap
Fine, i'll let the edit stand if we're just going to fight over it. What I find entertaining is that we find a four letter word more offensive than the two times I implied murder. Humans have some funny societal conventions. :)
Quibblesome
I think "He needed killin', your Honor" is still justifiable in my state. No jury of programmers would convict you ;-)
Steven A. Lowe
Not humans, specific cultures, imo. The implied murders are far more offensive to most people I know than any common swear word... even graphic sexual content would likely be much more acceptable than violence. Hurting and damaging people against their will is quite the monstrosity but consent-fully pleasuring people seems quite a beautiful act. So yes, cultures have some funny societal conventions (and on the internet we all collide :)
Oskar Duveborn
+5  A: 

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.

Eli
+17  A: 
try
{
  DoSomeImportantThing();
}
catch( Exception )
{
}

Not fun.

tkrehbiel
+4  A: 

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.

Mo
Reminds me a fellow developer who optimized web application by implementing his own sorting. Naturally this sped up page load several milliseconds so it was justified...
Petteri Hietavirta
+1  A: 

Code without tests (or testability)!

Kind Regards

marcospereira
+5  A: 

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.

Patrik
A: 

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

joel.neely
A: 
  • Premature optimization
  • Copy'n'Paste programming (copying instead of parameterizing/generalizing)
MP24
+4  A: 

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.

Alister Bulman
Love that site, reads like a simpler version of this place. Huge volume of good stuff. Worthwhile link, Sir.
Meff
+1  A: 

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;
  }
}
John Meagher
A: 

Putting implementation into the header file ... makes debugging impossible!

koschi
+1  A: 

Software vendor lock + golden hammer

A: 

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.

Crippledsmurf
+1  A: 

hardcoded strings (aka magical values)..

A: 

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.

myplacedk
A: 

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

Pradeep
+1  A: 

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.

M4N
A: 

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.

Igor Popov