views:

2969

answers:

11

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's an excellent discussion here. 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?

A: 

I believe the observer pattern has a lot to answer for, it works in very general cases, but as systems become more complex it becomes a nightmare, needing OnBefore(), OnAfter() notifications, and often posting of asynchronous tasks to avoid re-entrancy. A much better solution is to develop a system of automatic dependency analysis that instruments all object accesses (with read-barriers) during calculations and automatically creates an edge in a dependency graph.

Jesse Pepper
I understood everything in your answer up until the word "A"
1800 INFORMATION
You may need to expand or link to this automatic dependency analysis that you talk about. Also in .NET delegates/events are used instead of the observer pattern.
Spoike
@Spoike: delegates/events are an implementation of the observer pattern
orip
My personal grudge against Observer is that it can create memory leaks in garbage collected languages. When you have finished with an object you need to remember the object won't get cleaned up.
Martin Brown
@orip: yes, that's why you use delegates/events. ;)
Spoike
@Spoike: My suggested solution is very difficult to implement, but it's done once. We do it with a frontend preprocessor that turns references to objects into a call to a "read barrier" in which the object in question is automatically attached to by the framework, followed by the actual object access. When recalculations occur, dependency graphs are recalculated based on how far dirtiness has propagated through the graph.It's quite a complicated system to set up, but having done so, you don't have to worry about the observer pattern ever again!Note: needs to be thoroughly tested if u try
Jesse Pepper
I think the Observer pattern is actually underused b/c it's misunderstood. The Reactive Extensions for .NET is a great example of how to use the Observer pattern well.
Ryan Riley
+8  A: 

I believe the Template Method pattern generally is a very dangerous pattern.

  • A lot of times it uses up your inheritance hierarchy for "the wrong reasons".
  • Base classes have a tendency to become littered with all sorts of unerelated code.
  • It forces you to lock down design, often quite early in the development process. (Premature lock down in a lot of cases)
  • Changing this at a later stage becomes just harder and harder.
krosenvold
I would add that whenever you use Template Method you're probably better of using Strategy. The problem with TemplateMethod is that there is reentrancy between the base class and the derived class, which is often overly coupled.
Paul Hollingsworth
@Paul: Template method is great when used properly, i.e. when the parts that vary need to know a lot about the parts that don't. My take is that strategy should be used when the base code only calls the custom code and template method should be used when the custom code inherently needs to know about the base code.
dsimcha
yeah dsimcha, I agree... so long as the class designer is aware of this.
Paul Hollingsworth
+7  A: 

I think that Active Record is an overused pattern that encourages mixing business logic with the persistance code. It doesn't do a very good job of hiding the storage implementation from the model layer and ties the models to a database. There are plenty of alternatives (described in PoEAA) such as Table Data Gateway, Row Data Gateway and Data Mapper which often provide a better solution and certainly help to provide a better abstraction to storage. Also, your model should not need to be stored in a database; what about storing them as XML or accessing them using web services? How easy would it be to change the storage mechanism of your models?

That said, Active Record is not always bad and is perfect for simpler applications where the other options would be overkill.

Tim Wardle
Kinda true, but dependent to some extent on the implementation.
Mike Woodhouse
+79  A: 

Patterns are complex

All design patterns should be used with care. In my opinion you should refactor towards patterns when there is a valid reason to do so instead of implementing a pattern right away. The general problem with using patterns is that they add complexity. Overuse of patterns makes a given application or system cumbersome to further develop and maintain.

Most of the time, there is a simple solution, and you won't need to apply any specific pattern. A good rule of thumb is to use a pattern whenever pieces of code tend to be replaced or need to be changed often and be prepared to take on the caveat of complex code when using a pattern.

Remember that your goal should be simplicity and employ a pattern if you see a practical need to support change in your code.

Principles over patterns

It may seem like a moot to use patterns if they can evidently lead to over-engineered and complex solutions. However it is instead much more interesting for a programmer to read up on design techniques and principles that lay the foundation for most of the patterns. In fact one of my favorite books about on design patterns topic stresses this by reiterating on what principles are applicable on the pattern in question. They are simple enough to be useful than patterns in terms of relevance. Some of the principles are general enough to encompass more than object oriented programming (OOP), such as Liskov Substitution Principle, as long as you can build modules of your code.

There are a multitude of design principles but those described in the first chapter of GoF book are quite useful to start with.

  • Program to an 'interface', not an 'implementation'. (Gang of Four 1995:18)
  • Favor 'object composition' over 'class inheritance'. (Gang of Four 1995:20)

Let those sink in on you for a while. It should be noted that when GoF was written an interface means anything that is an abstraction (which also means super classes), not to be confused with the interface as a type in Java or C#. The second point principle comes from the observed overuse of inheritance which is sadly still common today.

From there you can read up on SOLID principles which was made known by Robert Cecil Martin (aka. Uncle Bob). Scott Hanselman interviewed Uncle Bob in a podcast about these principles:

  • Single Responsibility Principle
  • Open Closed Principle
  • Liskov Substitution Principle
  • Interface Segregation Principle
  • Dependency Inversion Principle

These principles are a good start to read up on and discussed together with your peers. You may find the principles to interweave with each other and other processes such as seperation of concerns and dependency injection. After doing TDD for a while you also may find that these principles come naturally in practice as you need to follow them to some degree in order to create isolated and repeatable unit tests.

Spoike
+1 Very good answer. It seems that every (newbie) programmer today knows his/her design patterns or at least know they exist. But very many never heard of, let alone apply, some of the absolutely essential principles like Single Responsibility to manage the complexity of their code.
eljenso
+2  A: 

A complement to Spoike's post, Refactoring to Patterns is a good read.

Adeel Ansari
I've actually linked to the book's catalog on the Internet. :)
Spoike
Oh! I didn't bother to hover it. Actually, right after finishing the question this book came into my mind, and then I saw your answer. I couldn't stop myself to post it. :)
Adeel Ansari
+11  A: 

Singletons - a class using singleton X has a dependency on it that's hard to see and hard to isolate for testing.

They're used very often because they're convenient and easy to understand, but they can really complicate testing.

See Singletons are Pathological Liars.

orip
They can also simplyfy testing as they can give you a single point to inject a Mock object. It is all down to getting balance right.
Martin Brown
@Martin: If course it's possible to change a singelton for testing (if you don't use the standard singelton implementation), but how is that easier than passing the test implementation in the constructor?
orip
+4  A: 

I hope I won't get beaten too much for this. Christer Ericsson wrote two articles (one, two) on the topic of design patterns in his real time collision detection blog. His tone is rather harsh, and perhaps a bit provocative, but the man knows his stuff, so I wouldn't dismiss it as ravings of a lunatic.

roe
Interesting reads. Thanks for the links!
Bombe
Morons produce bad code. Do morons with patterns produce worse code than morons that have never seen patterns? I don't think they do. For smart people, patterns provide a well-known vocabulary, which eases the exchange of ideas. The solution: Learn patterns and only deal with smart programmers.
Martin Brown
I don't think it is really possible for a true moron to produce worse code - no matter what tool they are using
1800 INFORMATION
+6  A: 

I don't think you should avoid Design Patterns (DP), and I don't think you should force yourself to use DPs when planning your architecture. We should only use DPs when they natural emerge from our planning.

If we define from the start that we want to use a given DP, many of our future design decisions will be influence by that choice, with no guarantee that the DP we chose is suited for our needs.

One thing we also shouldn't do is treat a DP as an immutable entity, we should adapt the pattern to our needs.

So, sumarizing, I don't think we should avoid DPs, we should embrace them when they are already taking shape in our architecture.

Megacan
+14  A: 

The one that the authors of Design Patterns themselves most worried about was the "Visitor" pattern.

It's a "necessary evil" - but is often over used and the need for it often reveals a more fundamental flaw in your design.

An alternative name for the "Visitor" pattern is "Multi-dispatch", because the Visitor pattern is what you end up with when you wish to use a single-type dispatch OO language to select the code to use based on the type of two (or more) different objects.

The classic example being that you have the intersection between two shapes, but there's an even simpler case that's often overlooked: comparing the equality of two heterogeneous objects.

Anyway, often you end up with something like this:

interface IShape
{
    double intersectWith(Triangle t);
    double intersectWith(Rectangle r);
    double intersectWith(Circle c);
}

The problem with this is that you have coupled together all of your implementations of "IShape". You've implied that whenever you wish to add a new shape to the hierarchy you will need to change all the other "Shape" implementations too.

Sometimes, this is the correct minimal design - but think it through. Does your design really mandate that you need to dispatch on two types? Are you willing to write each of the combinatorial explosion of multi-methods?

Often, by introducing another concept you can reduce the number of combinations that you're actually going to have to write:

interface IShape
{
    Area getArea();
}

class Area
{
    public double intersectWith(Area otherArea);
    ...
}

Of course, it depends - sometimes you really do need to write code to handle all of those different cases - but it's worth taking pause and having a think before taking the plunge and using Visitor. It might save you a lot of pain later on.

Paul Hollingsworth
Speaking about visitors, Uncle Bob uses it "all the time" http://butunclebob.com/ArticleS.UncleBob.IuseVisitor
Spoike
A: 

Iterator is one more GoF pattern to avoid, or at least to use it only when none of alternatives are available.

Alternatives are:

  1. for-each loop. This construction is present in most mainstream languages and may be used to avoid iterators in majority of cases.

  2. selectors à la LINQ or jQuery. They should be used when for-each is not appropriate because not all of objects from container should be processed. Unlike iterators, selectors allow to manifest in one place what kind objects is to be processed.

Volodymyr Frolov
I agree with selectors.Foreach is an iterator, most OO languages provide an iterable interface that you implement to allow a foreach.
Neil Aitken
In some languages for-each construction may be implemented through iterators, but concept of it is actually more high-level and closer to selectors. When using for-each developer declares explicitly that all of elements from container should be processed.
Volodymyr Frolov
Iterators are a great pattern. The anti-pattern would be implementing IEnumerable/IEnumerator _without_ an iterator. I believe LINQ was made possible through the `yield` iterator. Eric White has some great discussion on this in C# 3.0: http://blogs.msdn.com/b/ericwhite/archive/2006/10/04/the-yield-contextual-keyword.aspx. Also, check out Jeremy Likness' discussion on coroutines with iterators: http://www.wintellect.com/CS/blogs/jlikness/archive/2010/03/23/sequential-asynchronous-workflows-in-silverlight-using-coroutines.aspx.
Ryan Riley
@Ryan Riley, iterators are low level objects and thus they are to be avoid in high level design and code. Details of implementation of iterators and different kinds of selectors doesn't matter here.Selectors, unlike Iterators allow programmer to express explicitly what they want to process and so that they are high level.
Volodymyr Frolov
@Volodymyr I suppose we will have to disagree. I find the yield keyword very declarative, especially in languages like F#, Python, etc. where it is used in list comprehensions:`[ for x in xs do yield x.Value ] // F#``foreach (var x in xs) { yield x.Value; } // C#`True, `xs.Select(x => x.Value);` is much shorter and declarative, but that doesn't make list comprehensions "low level" or un-declarative.
Ryan Riley
Fwiw, the alternative, LINQ-like F# syntax is ` List.map (fun x -> x.Value) xs`, which is just about as long as the list comprehension.
Ryan Riley
@Ryan Riley we are talking about Iterators vs Selectors but not about yield keyword. It may be implemented via Iterators but not necessarily. Actually it is a separate idiom, just like for-each. More over, I'm talking about "other side" of Iterators. I'm talking not about producers of collections, but about code which consumes collections and iterates/select elements of it. From the consumers' point of view Selectors are declarative but Iterators are not.
Volodymyr Frolov
@Volodymyr: We appear to have different definitions of iterators. Can you point me to something to help me understand what you are talking about? I'm not sure how the consumer of an iterator is different than the consumer of a "selector," whatever that is.The Select combinator in LINQ is more commonly known as map in functional languages. It takes a collection and transforms each element, returning a new collection. `foreach` iterates/enumerates a collection without returning (unless you use `yield`).
Ryan Riley
@Ryan I'm talking about GoF definition of Iterator design pattern. And your understanding of Iterators and Selectors is correct, but note that both foreach and yield are separate idioms and may or may not be implemented via Iterators.
Volodymyr Frolov
@Ryan Consumer of Iterators must go over all elements in collection and filter some of them deep inside loop. Selectors in opposite to Iterators allow programmer to express what exactly should be processed at the very beginning of expression and perform `map` on selected elements after that.
Volodymyr Frolov
@Volodymyr I think I understand what you call selectors as filters (i.e. LINQ Where combinator). (Eureka - jQuery selectors! Missed that; sorry.) IIRC, underneath the hood, filters generally iterate over an entire collection returning the matched items. So they technically work the same. So you are saying don't build it from scratch; use a built-in idiom like foreach or LINQ. I agree with that. I don't think that means the design pattern is flawed, though.
Ryan Riley
@Ryan Iterators and Selectors may be implemented very the same, or Selectors may be implemented via Iterators, it doesn't mater. This doesn't mean that they are similar from the user point of view. The code which uses Selectors is much more expressive and declarative that the one which uses Iterators. And actually that does mean the design pattern is flawed (in comparison with Selector).
Volodymyr Frolov
+3  A: 

Some say that service locator is an anti pattern.

Arnis L.