I am studying patterns and anti-patterns. I have a clear idea about patterns, but I don&#39;t get anti-patterns. Definitions from the web and Wikipedia confuse me a lot.

Can anybody explain to me in simple words what an anti-pattern is? What is the purpose? What do they do? Is it a bad thing or good thing?



What is an anti-pattern?

All of us who work with relational databases have learned (or are learning) that SQL  is different. Eliciting the desired results, and doing so efficiently, involves a tedious process partly characterized by learning unfamiliar paradigms, and finding out that some of our most familiar  programming patterns don&#39;t work here. What are the common antipatterns you&#39;ve seen (or yourself committed)?

What are the most common SQL anti-patterns?

So we have this huge (is 11000 lines huge?) mainmodule.cpp source file in our project and every time I have to touch it I cringe.

As this file is so central and large, it keeps accumulating more and more code and I can&#39;t think of a good way to make it actually start to shrink.

The file is used and actively changed in several (&gt; 10) maintenance versions of our product and so it is really hard to refactor it. If I were to &quot;simply&quot; split it up, say for a start, into 3 files, then merging back changes from maintenance versions will become a nightmare. And also if you split up a file with such a long and rich history, tracking and checking old changes in the `SCC` history suddenly becomes a lot harder.

The file basically contains the &quot;main class&quot; (main internal work dispatching and coordination) of our program, so every time a feature is added, it also affects this file and every time it grows. :-(

What would you do in this situation? Any ideas on how to move new features to a separate source file without messing up the `SCC` workflow?

(Note on the tools: We use C++ with `Visual Studio`; We use `AccuRev` as `SCC` but I think the type of `SCC` doesn&#39;t really matter here; We use `Araxis Merge` to do actual comparison and merging of files)

What to do about a 11000 lines C++ source file?

**[anti-pattern][1]** : there must be at least two key elements present to formally distinguish an actual anti-pattern from a simple bad habit, bad practice, or bad idea:

 - Some repeated pattern of action, process or structure that initially appears to be beneficial, but ultimately produces more bad consequences than beneficial results, and
 - A refactored solution that is clearly documented, proven in actual practice and repeatable.

Vote for the TDD anti-pattern that you have seen &quot;in the wild&quot; one time too many.  
[The blog post by James Carr][2] and   [Related discussion on testdrivendevelopment yahoogroup][3]

If you&#39;ve found an &#39;unnamed&#39; one.. post &#39;em too. **One post per anti-pattern please** to make the votes count for something.

*My vested interest is to find the top-n subset so that I can discuss &#39;em in a lunchbox meet in the near future.*


  [1]: http://en.wikipedia.org/wiki/Anti-pattern
  [2]: http://blog.james-carr.org/2006/11/03/tdd-anti-patterns/
  [3]: http://tech.groups.yahoo.com/group/testdrivendevelopment/message/20745

Unit testing Anti-patterns catalogue

I was reading the Singleton article on Wikipedia and I came across this example:

    public class Singleton {
    	// Private constructor prevents instantiation from other classes
    	private Singleton() {}
    
    	/**
    	 * SingletonHolder is loaded on the first execution of Singleton.getInstance() 
    	 * or the first access to SingletonHolder.INSTANCE, not before.
    	 */
    	private static class SingletonHolder { 
    		private static final Singleton INSTANCE = new Singleton();
    	}
    
    	public static Singleton getInstance() {
    		return SingletonHolder.INSTANCE;
    	}
    }

While I really like the way this Singleton behaves, I can&#39;t see how to adapt it to incorporate arguments to the constructor.  What is the preferred way to do this in Java?  Would I have to do something like this?

    public class Singleton
    {
    	private static Singleton singleton = null;  
    	private final int x;
    
    	private Singleton(int x) {
    		this.x = x;
    	}
    
    	public synchronized static Singleton getInstance(int x) {
    		if(singleton == null) singleton = new Singleton(x);
    		return singleton;
    	}
    }

Thanks!


----------


Edit:  I think I have started a storm of controversy with my desire to use Singleton.  Let me explain my motivation and hopefully someone can suggest a better idea.  I am using a grid computing framework to execute tasks in parallel.  In general, I have something like this:

    // AbstractTask implements Serializable
    public class Task extends AbstractTask
    {
    	private final ReferenceToReallyBigObject object;
    
    	public Task(ReferenceToReallyBigObject object)
    	{
    		this.object = object;
    	}
    
    	public void run()
    	{
    		// Do some stuff with the object (which is immutable).
    	}
    }

What happens is that even though I merely pass a reference to my data to all of the tasks, when the tasks are serialized, the data gets copied over and over.  What I want to do is share the object among all of the tasks.  Naturally, I might modify the class like so:

    // AbstractTask implements Serializable
    public class Task extends AbstractTask
    {
    	private static ReferenceToReallyBigObject object = null;
    
    	private final String filePath;
    
    	public Task(String filePath)
    	{
    		this.filePath = filePath;
    	}
    
    	public void run()
    	{
    		synchronized(this)
    		{
    			if(object == null)
    			{
    				ObjectReader reader = new ObjectReader(filePath);
    				object = reader.read();
    			}
    		}
    
    		// Do some stuff with the object (which is immutable).
    	}
    }

As you can see, even here I have the issue that passing a different file path means nothing after the first one is passed.  This is why I like the idea for a *store* which was posted in the answers.  Anyhow, rather than including the logic for loading the file in the run method, I wanted to abstract this logic into a Singleton class.  I will not provide yet another example, but I hope you get the idea.  Please let me hear your ideas for a more elegant way to accomplish what I am trying to do.  Thank you again!

Singleton with Arguments in Java

Recently I&#39;ve read [Mark Seemann&#39;s article](http://blog.ploeh.dk/2010/02/03/ServiceLocatorisanAnti-Pattern/) about Service Locator anti-pattern.

Author points out two main reasons why ServiceLocator is an anti-pattern:

1. **API usage issue** (which I&#39;m perfectly fine with)  
 When class employs a Service locator it is very hard to see its dependencies as, in most cases, class has only one PARAMETERLESS constructor.
 In contrast with ServiceLocator, DI approach explicitly exposes dependencies via constructor&#39;s  parameters so dependencies are easily seen in IntelliSense.
 
2. **Maintenance issue** (which puzzles me)  
Consider the following expample  

We have a class *&#39;MyType&#39;* which employs a Service locator approach:  

&lt;!-- language-all: lang-cs --&gt;

    public class MyType
    {
    	public void MyMethod()
    	{
    		var dep1 = Locator.Resolve&lt;IDep1&gt;();
    		dep1.DoSomething();
    	}
    }

Now we want to add another dependency to class &#39;MyType&#39;
	
    public class MyType
    {
    	public void MyMethod()
    	{
    		var dep1 = Locator.Resolve&lt;IDep1&gt;();
    		dep1.DoSomething();
    			
    		// new dependency
    		var dep2 = Locator.Resolve&lt;IDep2&gt;();
    		dep2.DoSomething();
    	}
    }

And here is where my misunderstanding starts. The author says: 

&gt; It becomes a lot harder to tell whether you are introducing a breaking change or not. You need to understand the entire application in which the Service Locator is being used, and the compiler is not going to help you.

But wait a second, if we were using DI approach, we would introduce a dependency with another parameter in constructor (in case of constructor injection). And the problem will be still there. If we may forget to setup ServiceLocator, then we may forget to add a new mapping in our IoC container and DI approach would have the same run-time problem.

Also, author mentioned about unit test difficulties. But, won&#39;t we have issues with DI approach? Won&#39;t we need to update all tests which were instantiating that class? We will update them to pass a new mocked dependency just to make our test compilable. And I don&#39;t see any benefits from that update and time spending.

I&#39;m not trying to defend Service Locator approach. But this misunderstanding makes me think that I&#39;m losing something very important. Could somebody dispel my doubts?



**UPDATE (SUMMARY):**

The answer for my question &quot;Is Service Locator an anti-pattern&quot; really depends upon the circumstances. And I definitely wouldn&#39;t suggest to cross it out from your tool list. It might become very handy when you start dealing with legacy code. If you&#39;re lucky enough to be at the very beginning of your project then the DI approach might be a better choice as it has some advantages over Service Locator.

And here are main differences which convinced me to not use Service Locator for my new projects:

 - Most obvious and important: Service Locator hides class dependencies
 - If you are utilizing some IoC container it will likely scan all constructor at startup to validate all dependencies and give you immediate feedback on missing mappings (or wrong configuration); this is not possible if you&#39;re using your IoC container as Service Locator

For details read excellent answers which are given below.

Is ServiceLocator an anti-pattern?

I&#39;ve recently overheard people saying that [data transfer objects][1] (DTOs) are an *anti-pattern*. 

Why? What are the alternatives?

  [1]: https://en.wikipedia.org/wiki/Data_transfer_object


Why are data transfer objects (DTOs) an anti-pattern?

A common antipattern in Python is to concatenate a sequence of strings using `+` in a loop. This is bad because the Python interpreter has to create a new string object for each iteration, and it ends up taking quadratic time. (Recent versions of CPython can apparently optimize this in some cases, but other implementations can&#39;t, so programmers are discouraged from relying on this.) `&#39;&#39;.join` is the right way to do this.

However, I&#39;ve heard it said ([including here on Stack Overflow](https://stackoverflow.com/questions/10001301/list-slicing-python/10001401#10001401)) that you should _never, ever_ use `+` for string concatenation, but instead always use `&#39;&#39;.join` or a format string. I don&#39;t understand why this is the case if you&#39;re only concatenating two strings. If my understanding is correct, it shouldn&#39;t take quadratic time, and I think `a + b` is cleaner and more readable than either `&#39;&#39;.join((a, b))` or `&#39;%s%s&#39; % (a, b)`.

Is it good practice to use `+` to concatenate two strings? Or is there a problem I&#39;m not aware of?

Any reason not to use &#39;+&#39; to concatenate two strings?

A lot of people seem to agree, that the Singleton pattern has a number of drawbacks and some even suggest avoiding the pattern entirely. There&#39;s an [excellent discussion here][1]. Please direct any comments about the Singleton pattern to that question. 

**My question**: Are there other design patterns, that should be avoided or used with great care?

  [1]: https://stackoverflow.com/questions/137975/what-is-so-bad-about-singletons

Design patterns to avoid

This question was sparked by a discussion around [this article](http://ayende.com/blog/30721/review-microsoft-n-layer-app-sample-part-vndash-cross-cutting-is-a-fine-line), where I did not receive any good answers. 

*Why should logging your exception and then rethrowing it (preserving the original stack trace of course) be a bad idea if you can&#39;t handle it otherwise?*

Why is &quot;log and throw&quot; considered an anti-pattern?

&gt; **Possible Duplicate:**  
&gt; [What is so bad about Singletons?](https://stackoverflow.com/questions/137975/what-is-so-bad-about-singletons)
&gt;
&gt; [Singleton Design Pattern: Pitfalls](https://stackoverflow.com/questions/1448393/singleton-design-pattern-pitfalls)  
&gt;
&gt; [Singleton anti-pattern](https://stackoverflow.com/questions/12075993/singleton-anti-pattern)  

&lt;!-- End of automatically inserted text --&gt;

I&#39;ve heard recently that Singleton is an anti-pattern. I know it has to do with the fact making a class singleton is like making that unique instance a global variable, but it&#39;s also doing a lot more than that (limiting the number of instances of that object, managing instantiation, etc..).

Why exactly is Singleton considered an anti-pattern? And what are the alternatives?

Why is Singleton considered an anti-pattern?


What is the most evil or dangerous code fragment you have ever seen in a production environment at a company?  I&#39;ve never encountered production code that I would consider to be deliberately malicious and evil, so I&#39;m quite curious to see what others have found.

The most dangerous code I have ever seen was a stored procedure two linked-servers away from our core production database server.  The stored procedure accepted any NVARCHAR(8000) parameter and executed the parameter on the target production server via an double-jump sp_executeSQL command.  That is to say, the sp_executeSQL command executed another sp_executeSQL command in order to jump two linked servers.  Oh, and the linked server account had sysadmin rights on the target production server.

What is the most EVIL code you have ever seen in a production enterprise environment?

I discovered this pattern (or anti-pattern) and I am very happy with it.

I feel it is very agile:

    def example():
        age = ...
        name = ...
        print &quot;hello %(name)s you are %(age)s years old&quot; % locals()

Sometimes I use its cousin:

    def example2(obj):
        print &quot;The file at %(path)s has %(length)s bytes&quot; % obj.__dict__

I don&#39;t need to create an artificial tuple and count parameters and keep the %s matching positions inside the tuple.

Do you like it? Do/Would you use it? Yes/No, please explain.

Python: is using &quot;..%(var)s..&quot; % locals() a good practice?

Is it possible to block users from closing the window using the exit button &lt;kbd&gt;[X]&lt;/kbd&gt;? I am actually providing a close button in the page for the users to close the window.Basically what I&#39;m trying to do is to force the users to fill the form and submit it. I don&#39;t want them to close the window till they have submitted it.

I really appreciate your comments, I&#39;m not thinking of hosting on any commercial website. Its an internal thing, we are actually getting all the staff to participate in this survey we have designed....
 
I know its not the right way but I was wondering if there was a solution to the problem we have got here...

How to block users from closing a window in Javascript?

Subversion lets you embed working copies of other repositories using [externals][1], allowing easy version control of third-party library software in your project.

While these seem ideal for the reuse of libraries and version control of [vendor software][2], they aren&#39;t without [their critics][3]:

&gt;Please don&#39;t use Subversion externals (or similar in other tools), they are an anti-pattern and, therefore, unnecessary

Are there hidden risks in using externals? Please explain why they would they be considered an antipattern.


  [1]: http://svnbook.red-bean.com/en/1.1/ch07s04.html
  [2]: http://svnbook.red-bean.com/en/1.1/ch07s05.html
  [3]: https://stackoverflow.com/questions/222827/how-do-you-organize-your-version-control-repository#304036

Are subversion externals an antipattern?

How does one implement the Singleton design pattern in the go programming language?

Singleton in go

I have been told not to append stuff using `element.innerHTML += ...` like this:

    var str = &quot;&lt;div&gt;hello world&lt;/div&gt;&quot;;
    var elm = document.getElementById(&quot;targetID&quot;);
    
    elm.innerHTML += str; //not a good idea?

What is wrong with it?, what other alternatives do I have?

Why is &quot;element.innerHTML+=&quot; bad code?

I find that what not to do is a harder lesson to learn than what should be done. 

From my experience, what separates an expert from an intermediate is the ability to select from among various seemingly equivalent ways of doing the same thing.

So, when it comes to JavaScript **what kinds of things should you not do and why**?

I&#39;m able to find lots of these for Java, but since JavaScript&#39;s typical context (in a browser) is very different from Java&#39;s I&#39;m curious to see what comes out.

What anti-patterns exist for JavaScript?

To cut a long story short: I find the [Java antipatterns][1] an indispensable resource. For beginners as much as for professionals. I have yet to find something like this for C#. So I&#39;ll open up this question as community wiki and invite everyone to share their knowledge on this. As I am new to C#, I am strongly interested in this, but cannot start with some antipatterns :/

Here are the answers which I find specifically true for C# and not other languages.

*I just copy/pasted these! Consider throwing a look on the comments on these as well.*

----------------------------------------------------------------------------------------

# [Throwing `NullReferenceException`][5]

Throwing the wrong exception:

    if (FooLicenceKeyHolder == null)
        throw new NullReferenceException();

----------------------------------------------------------------------------------------

# [Properties vs. public Variables][2]
Public variables in classes (use a property instead).

**Unless** the class is a simple Data Transfer Object.

----------------------------------------------------------------------------------------

# [Not understanding that bool is a real type, not just a convention][3]

    if (myBooleanVariable == true)
    {
        ...
    }

or, even better

    if (myBooleanVariable != false)
    {
        ...
    }

Constructs like these are often used by `C` and `C++` developers where the idea of a boolean value was just a convention (0 == false, anything else is true); this is not necessary (or desirable) in C# or other languages that have real booleans.

----------------------------------------------------------------------------------------

# [Using `using()`][4]

Not making use of `using` where appropriate:

    object variable;
    variable.close(); //Old code, use IDisposable if available.
    variable.Dispose(); //Same as close.  Avoid if possible use the using() { } pattern.
    variable = null; //1. in release optimised away.  2. C# is GC so this doesn&#39;t do what was intended anyway.


[1]: http://www.odi.ch/prog/design/newbies.php
[2]: https://stackoverflow.com/questions/1529604/c-antipatterns/1529619#1529619
[3]: https://stackoverflow.com/questions/1529604/c-antipatterns/1529797#1529797
[4]: https://stackoverflow.com/questions/1529604/c-antipatterns/1529636#1529636
[5]: https://stackoverflow.com/questions/1529604/c-antipatterns/1529674#1529674

C# Antipatterns

I&#39;m a bit confused about how to best refactor my code into something more readable.

Consider this piece of code:

    var foo = getfoo();
    if(foo!=null)
    {
        var bar = getbar(foo);
        if(bar!=null)
        {
            var moo = getmoo(bar);
            if(moo!=null)
            {
                var cow = getcow(moo);
                ...
            }
        }
    }
    return;

As you can see, lots of nested `if` blocks, needed because each nested if is dependent on the previous values.

Now I was wondering how to make my code a bit cleaner in this regard.

Some options I have thought of myself would be to:

 - return after each if clause, meaning I&#39;d have multiple places where I leave my method
 - throw `ArgumentNullException`s, after which I&#39;d catch them in the end and place the return statement in my finally clause (or outside the try/catch block)
 - work with a label and `goto:`

Most of these options seem a bit &quot;dirty&quot; to me, so I was wondering if there was a good way to clean up this mess that I&#39;ve created.


All Anti Patterns Solutions on Gang of Coders

Total of 20 Anti Patterns Solutions