Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

What are the worst anti-patterns you have came across in your career as a programmer?

I'm mostly involved in java, although it is probably language-independent.

I think the worst of it is what I call the main anti-pattern. It means program consisting of single, extremely big class (sometimes accompanied with a pair of little classes) which contains all logic. Typically with a big loop in which all business logic is contained, sometimes having tens of thousands of lines of code.

share

locked by World Engineer Sep 7 '13 at 6:21

This question exists because it has historical significance, but it is not considered a good, on-topic question for this site, so please do not use it as evidence that you can ask similar questions here. This question and its answers are frozen and cannot be changed. More info: help center.

closed as primarily opinion-based by World Engineer Sep 7 '13 at 6:21

Many good questions generate some degree of opinion based on expert experience, but answers to this question will tend to be almost entirely based on opinions, rather than facts, references, or specific expertise.If this question can be reworded to fit the rules in the help center, please edit the question.

comments disabled on deleted / locked posts

26 Answers

Commented out code. Blocks of it, maybe hundreds of lines. The theory being, hey it is commented out, it doesn't do any harm and maybe we will need it in the future.

share
7  
Well at least you can safely delete it. Unlike someone I've worked with that would frequently rename procedures and leave the dead code accessible. Blech. –  Cyrena Feb 4 '11 at 18:25
9  
This is another case where using version control pays off. You can delete code without having to ask whether or not you may need it in the future because it can be retrieved again with a few keystrokes. –  Jason Feb 4 '11 at 20:28
5  
I think commented-out code has a place, if you prefix it with a comment that explains why its there. Sometimes I will leave a commented block that represents a potential road I was thinking about taking, and leave it with a comment about why I didn't. @Jason, I definitely hear you about this being the role of version control, but deleted code in version control is not very discoverable. –  nlawalker Feb 4 '11 at 20:42
show 5 more comments

I'll hit another obvious one with "copy-pasta". Copying code that is nearly identical to what you want, then changing a few things (instead of extracting into a method).

This is especially prevalent in some functional and API test code from the late 90's: Literally hundreds (or even thousands) of nearly-identical test cases that could have been broken out into a few functions that take 3 or 4 parameters - or, even better, something data-driven. My first job out of college was literally 8 months of rewriting and refactoring thousands of lines of copy-pasta that were using deprecated methods. By the time I was done, the test files were less than a 10th of their original size and far more maintainable (and readable!).

share
add comment

I think that I can write a lot about Pattern Mania and solutions that could be solved with a lot less effort, but I would rather point to a great article that I've read recently with a fantastic example of how a simple solution can be overcomplicated.

How (not) to write Factorial in Java or so we put a factory into your algorithm

share
3  
Awesome! Now where's the FactorialWebService? :P –  FrustratedWithFormsDesigner Feb 4 '11 at 18:26
1  
I call it Pattern Fever...everyone gets it at one point in their career. The better among us grow beyond it. I had an interview where the guy asked me when should I think about patterns. I told him that I let them evolve into the system. He said "No, you should use patterns from the start." –  Mike Brown Feb 4 '11 at 18:30
show 3 more comments

Golden Hammer

“I have a hammer and everything else is a nail.”

share
add comment

Regions

In C# you can define a region of code that can be collapsed in the IDE, thus hiding it unless you want to deal with that code. I've been on (am currently on) a project where the regions spanned hundreds of lines (wish I was exaggerating) and there were multiple regions in a thousand line function (again I wish I was kidding).

On the upside, the developer who made the region did a very good job at identifying specific functions within a region. So much so that I was able to do an extract method on the region and move on.

Regions encourage developers to "hide" their crap in plain sight.

share
15  
+1 for Regions encourage developers to "hide" their crap in plain sight. However, when used properly, I find that regions help to logically group together code and easily find at a later time. –  Darren Young Feb 4 '11 at 19:17
2  
I love regions...but only for organization...not hiding digital reams of code. –  IAbstract Feb 4 '11 at 20:31
3  
+1. That's also why using regions violates StyleCop rule SA1124 DoNotUseRegions. –  MainMa Feb 4 '11 at 21:01
1  
I have projects which work with a huge number of fields in SQL, and the code that updates/creates it is many lines. A region #region SQL Update makes it collapsible so less scrolling is required. –  JYelton Feb 5 '11 at 0:13
show 1 more comment

To me the worst pattern is Copy/Paste.

share
28  
To me the worst pattern is Copy/Paste. –  Andrew Arnold Feb 4 '11 at 20:44
add comment

Not overloading methods:

// Do something
public int doSomething(Data d);

// Do same thing, but with some other data
public int doSomething2(SomeOtherData d);

Clearly shows that the programmer did not understand overloading.

share
add comment

Loop Switch Sequences

http://en.wikipedia.org/wiki/Loop-switch_sequence

They annoy me to no end and really show how inexperienced the dev was

share
show 2 more comments

Soft coding, ie when programmers go out of their way to avoid hard-coding and end up at the other side of the scale - "hard coded" dependencies.

share
add comment

Keep state in the client.

Once worked with a web application where all state was kept in the browser. (To ensure trouble-free scalability all exceptions were ignored).

share
show 3 more comments

Massive Checkins

I hate when I see a developer hasn't done a check-in in over a week. It means he's either stuck and hasn't looked for help, or he's clumping a bunch of features into one big check-in. (I left out the worst case scenario, he just isn't doing anything. That's easy to resolved...two words sounds like you're hired.)

If you're making a big check-in you lose a lot of benefits of SCM, like being able to link a changeset to a specific feature. Also you're likely to have a lot of merging to do and that's not necessarily easy to get right.

share
1  
Not doing anything is not always the worst case scenario. Sometimes it's better to have to write it from scratch than to have code to rewrite... –  Malachi Aug 25 '11 at 14:10
add comment

Currently, I'm working with a piece of legacy code and I love the way that previous coder gets the first element of a list:

String result;
for(int i = 0; i < someList.size(); i++) {
    result = someList.get(i);
    break;
}

But imho worst I've seen in this code is defining classes inline JSPs pages, write all the HTML, CSS and Javascript using scriptlet and out.println :-(

share
add comment

One of the worst behavioral anti patterns I have seen was a shop that only allowed code to be checked in to version control after it was in production. Coupled with exclusive checkouts in VSS, this led to a convoluted process of undoing checkouts if a production defect needed to be corrected before the next release, let alone more than one person needing to change a given file for the upcoming release.

The fact that this was a departmental policy made it even worse than the case of a single developer behaving this way.

share
add comment

Locking on a String literal

synchronized("one") { /* block one A*/ }

synchronized("one") { /* block one B*/ }

Very long class names. (in the JRE)

com.sun.java.swing.plaf.nimbus.
InternalFrameInternalFrameTitlePaneInternalFrameTitlePaneMaximizeButtonWindowNotFocusedState

Poor inheritance structure

com.sun.corba.se.internal.Interceptors.PIORB extends
com.sun.corba.se.internal.POA.POAORB extends
com.sun.corba.se.internal.iiop.ORB extends
com.sun.corba.se.impl.orb.ORBImpl extends
com.sun.corba.se.spi.orb.ORB extends
com.sun.corba.se.org.omg.CORBA.ORB extends 
org.omg.CORBA_2_3.ORB extends
org.omg.CORBA.ORB

An Exception which is not.

public interface FlavorException { }

Pointless and cryptic error handling

if (properties.size() > 10000)
   System.exit(0);

Unnecessary creation of objects

Class clazz = new Integer(0).getClass();
int num = new Integer(text).intValue();

Throwing an exception for other purposes

try {
    Integer i = null;
    Integer j = i.intValue();
} catch (NullPointerException e) {
    System.out.println("Entering "+e.getStackTrace()[0]);
}

Using instance objects for static methods.

Thread.currentThread().sleep(100);

Synchronizing on a non-final field

synchronized(list) {
   list = new ArrayList();
}

Pointless copy of a constant String

String s = new String("Hello world");

Pointless call to String.toString()

String s = "Hello";
String t = s.toString() + " World";

Calls to System.gc() to free some memory

Setting local variable to null to free some memory

    // list is out of scope anyway.
    list = null;
}

Using ++i instead of i++ for performance reasons (or any other micro-micro-optimisation)

share
1  
@Peter: "Calls to System.gc() to free some memory", I have seen one occasion where .NET would explode with OutOfMemory exception unless GC was called explicitly. Of course it was more of an exception rather than a rule. –  Coder Aug 25 '11 at 21:49
show 2 more comments

Code sprinkled with:

// TODO: 

or

// TODO: implement feature 

with no additional information.

share
add comment

Iterator for dummies:

Iterator iCollection = collection.iterator();
for(int i = 0 ; i < collection.size() ; i++ ){
    if(collection.get(i) == something){
       iCollection.remove();
    }
 }

Singletono-Factory:

public class SomeObject{
   private SomeObject() {}
   public static SomeObject getInstance(){
       return new SomeObject();
   }
}

if-else driven development (also called open close principle - open for modification close for understanding):

if (sth1){
...  
}else if(sth2){
..
}
...
..
else if(sth1000000000000){
...
}

StringPattern (also called StringObject):

a) sendercode
if(sth1)
   str+="#a";
if(sth2)
   str+="#b";
...
if(sth1000)
   str+="#n";

b) receiver
   regexp testing if str contains #a, #b, ... #n
share
2  
The singleton is a factory. There is nothing wrong with that code. The only thing that could be wrong is that outside code makes the assumption, that every call to getInstance returns the same instance. Such an assumption would break encapsulation and is in fact one of the most common anti-patterns. –  back2dos Aug 25 '11 at 14:59
show 3 more comments

It's not so much of a coding pattern but a behavioural pattern, it's quite bad though: modifying something in the code (let's say the requirements changed), then tweaking all the unit tests until the code passes it. Tweaking or simply removing all test code from the test method, but leaving the method there.

This is linked to a more generic one though, the That'll Do pattern, here's a representative line of code for it:

int num = new Integer( stringParam ).parseInt( stringParam );

It works, after all.

share
add comment

I absolutely despise abstraction inversion, or the reinventing of low-level primitives on top of high-level primitives. Sometimes, though, this is caused by bad language designers, not bad programmers. Examples:

  1. Using a single method class with no member variables and a corresponding interface, (implemented in terms of tables of function pointers), instead of a function pointer. Note that in languages like Java, you may have no choice.

  2. In MATLAB and R, the insistence that everything is a vector/matrix rather than a primitive.

  3. While we're bashing MATLAB, how about the fact that it has no integers, so when you need an integer you have to use a double.

  4. Purely functional languages where you have to use function calls to write a loop.

share
add comment

It would be definitely copy/paste, I've seen a lot of bad code getting done because of it, that and cowboy coding and everything that derives from that. (God Classes, extra large methods, bad thinked algorithms, etc.)

And if design patterns are allowed, them I would say: Doing a project as a set of actions from a plan, without doing any design or domain analysis.

share
add comment

Multi-use java bean -

A java bean with lots of variables, used in a few different kinds of operations. Each operation uses some arbitrary subset of the bean variables, and ignores the others. A few variables for gui state, a few variables thrown in for passing around between components, a few that probably aren't even used anymore. Best examples have no documentation, which would hinder appreciation of the pattern.

Also, can't forget the beloved

try{ 
   ... //many lines of likely dangerous operations
}
catch(Exception e){}
share
show 2 more comments

A former co-worker had the habit of re-using objects by overwriting their properties, instead of simply creating new ones. I could never have imagined the amount of problems this has caused when implementing new features.

share
add comment

Something that has caused me lots of grief is the "Big map in the sky" pattern. Throwing around a map instead of using proper objects. You have no idea of what "variables" it contains without debugging, and you don't know what it might contain without tracing the code backwards. Typically maps Strings to Objects, or Strings to Strings, which you potentially are supposed to parse into primitives.

share
show 1 more comment

One of my favorite development anti patterns is the use of a database "design" that requires continually adding additional columns to one or more tables programmatically. This is a cousin of the "design" that creates a new table for each instance of an entity. Both inevitably hit limitations of the database server, but usually not until the system has been in production for quite some time.

share
add comment

I think one of the worst anti-patterns I have seen involves using a Database table as temporary storage instead of using computer memory.

The problem domain is proprietary which disallows me from explaining it, but it isn't necessary to understand the basic problem. This was a GUI application written in Java with a backend Database. It was to take certain input data, manipulate it and then commit the processed data to the database.

Our project has a fairly complicated algorithm which saves intermediate values for processing later on. Instead of encapsulating the temporary objects in... objects, a database table was created like so "t_object". Every time a value was calculated, it was added to this table. After the algorithm finished it's work, it would select out all of the intermediate values and process them all in one large Map object. After all the processing had been finished, the remaining values that were marked to be saved would be added to the real database schema and the temporary entries in the "t_object" table would be thrown away.

The table was also used like a unique list, data could only exist once. This might have been a decent feature of the design had we implemented Constraints on the table, but we ended up iterating through the whole table to see if the data existed or not. (No we didn't even use queries that used where clauses with CONTAINS)

Some of the problems we ran into due to this design were specifically debugging. The application was built to pipeline data so there would be multiple GUIs that would pre-process the data before it arrived at this algorithm. The debugging process was to process a test case and then pause right after finishing the above section. Then we would query the database to see what data was contained in this table.

Another problem we discovered was that data wasn't being deleted properly from this temporary table, which would interfere with run's in the future. We discovered this was due to Exceptions not being properly handled and therefore the application not exiting properly and not deleting the data in the table it was in control of.

If we had used Basic Object-Oriented Design and kept everything in Memory, these above problems would never have occurred. First, debugging would have been simple as we could easily setup break points in the application and then inspect the memory in the stack and heap. Secondly, upon abnormal exit from the application, java memory would have been cleaned up naturally without having to worry about deleting it from the database.

NOTE: I'm not saying this pattern is inherently bad, but in this example I found it to be unnecessary when basic OO principles would have sufficed.

I'm not sure of a name for this anti-pattern as this is the first time I have seen anything like this done. Any good names you guys can think of for this pattern?

share
2  
You don't mention your problem domain, but not always a bad thing, keeping state elsewhere like this could allow processes to scale and to be introspected during long run times, also to help in debugging. Was the DB access causing performance issues or concerns? –  Xepoch Feb 4 '11 at 20:43
show 3 more comments

Cart-Before-The-Horse -- aka YouMightNeedIt

For example:

  • Creating a RDMBs schema with abstract concepts, cross references -- in essence an over-generalized data cube... And THEN writing features around a does-everything model.
share
show 1 more comment

IMO the worst anti-pattern I've seen is the "We don't need any stinkin' patterns" Anti-Pattern: The idea that design patterns are wastes of time and you can write code faster by just slopping it together and copy/pasting as needed.

Honorable mention goes to having code that loads an object from the database using the old VB6 style of:

Foobar oFoo = new Foobar();
oFoo.FooID = 42;
if (oFoo.Load()) { 
    // do something with oFoo
}

not really an anti-pattern, per se, but shows a lack of taking advantage of proper architecture and separation of concerns.

Also, things like this:

// this name is misleading, we may not always want to stand in fire,
// we may want to stand in slime or voidzones or ice patches...
public Foobar StandInFire() { }

// why is this here???
public string BeatWithNerfBat(string whom) { }

// ????
public int GivePony(string to) { }
share
add comment

Not the answer you're looking for? Browse other questions tagged or ask your own question.