Misused design patterns

Question

Are there, in the canonical Gang of Four list, any design patterns that you often find misused, misunderstood or overused (other than the highly debated Singleton)? In other words, is there a design pattern you would advise to think twice before using? (And why?)

Answer 1

Factory Patterns...

I was parachuted into a project before where every single MyObject in the system had an equivalent MyObjectFactory for generating new instances. There was no concept of abstraction or extended classes... just plain old ClassX & ClassXFactory.

And no-one could explain why... "It was just the way things had always been done"

Answer 2

The singleton pattern .. global state often leads to problems when testing

Any code depending on the singleton gets harder and harder to test because that dependency isn't easily mocked..

Answer 3

The only one (besides the aforementioned Singleton and its partner in crime, the Factory) wouldn't be a GoF, it would be setters and getters when applied to an object's native properties.

Setters and getters applied to member variables are functionally identical to public member variables. A getter without a setter is more like a public final member variable--but at that point why not just use a public final member variable, they do no more harm...

The only difference is that you "could" intercept the call and override it, but people rarely do. More often it's used as a crutch for procedural programmers to avoid OO programming (which is the real reason it's an anti-pattern).

With a setter and/or getter you are still exposing your inner member structure to the outside world (for instance, you'll have to refactor other classes if you find you need to change a int to a long) and you are almost assuring that some code that should be inside your object is instead being placed outside.

There are a few exceptions I can think of:

Setters used to simplify an objects construction. Sometimes it's necessary to create an object then set other values in afterwards. These values should be immutable (you shouldn't be able to call set twice) for safety.

Getters used to access contained objects. Since the contained objects are usually able to insure their own integrity, sharing them is great. Setters are generally bad in this case, you don't want an object with a specific state swapped-out right underneath your nose, it makes assuring your own integrity much more difficult.

Java Beans used for screen components: Yeah, can't figure out a better way to implement these "property balls". Reflection comes in handy for this component, the patterns are useful--it's kinda hacky but works.

DAO/DTO Bean objects. Honestly I think these are an iffy usage of the pattern, but they are the pattern. It makes manipulation of the properties via meta-data instead of code much more difficult than it should be since it has to be reflective. The beans properties are always tied to some outside source (database format, data transfer format, component properties, ...) so why are we duplicating the work of defining each part?

Edit: Stolen from kyoryu's comment, brought up to the post because It's really a perfect summary of what I was saying and could be missed in the comments. Needed since not everybody seems to get the concept that adding accessors to the language only codifies a bad OO design pattern:

Short version -

if (account1.balance > 1000)
{
    account1.balance = account1.balance - 1000;
    account2.balance = account2.balance + 1000;
}; = BAD CODE. 

account2.deposit(account1.withdraw(1000)); = GOOD CODE. 

The second one doesn't require accessors... – kyoryu (Slightly modified by bill k because I have a little more room than he did in his comment).

The second one moves the test and some other math inside Account rather than duplicating it throughout the code every place you might make a transfer.

Just to belabor the point EVEN MORE, note that with the "GOOD CODE" style it's pretty obvious that the output of .withdraw could be a Transaction object that contains information about the entire transaction including its success, source and destination and logging ability. How would this have occurred to someone who writes their code in "BAD CODE" style?

Also how would you refactor BAD CODE to even use such an object? It's just a mess.

Answer 4

I would also say the factory pattern. Similar experience as Eoin. In my case the project had tons of factories because some people thought you might have used object A for a local implementation and object B for remote one and it was abstracted via a factory (which is a sensible thing to do).

But the "remote" implementation has never been needed or implemented or even foreseen in the future... and also, less-skilled engineers started adopting the pattern for lots of other things just as a cookie cutter...

Answer 5

Actually, what I see most often is the lack of use of an appropriate pattern. Typical scenario: me: "Hey, module A already has a piece of code that loops through a set of objects and performs database operation X on them, why didn't you reuse that code?" coder: "well, but I had to do operation Y on those objects." me: "what about using refactoring it to use the Command pattern to execute X or Y as appropriate?"

I once saw usage of the Subject-Observer pattern get out of hand. It was implemented between processes using the database to persistently store the Subject. Because of the sheer number of updates to the subject, and the number of observers, the load on the database was tremendous, and caused an unforeseen system-wide slowdown.

Answer 6

The Mediator pattern definitely has the potential to be misused if all sorts of logic gets globbed into one huge class.

Answer 7

ALL.

Don't take me wrong here, I find them to be a great base and when understood well very helpful. It takes so much out of you to acquire the design skills to know well when and how to apply them in the code. Actually, that's the overall case for the skills to create clean code.

Its not about not using then, is exactly what you said in the question "think twice before using". Understand well what you are doing and why. If you don't, talk to someone that can coach you on that - besides reading a lot. Beware of the ones that know all the patterns but can't explain you clearly the what/why of any of them - specially of the one in question.

Answer 8

Actually, I would say design patterns in general are overused when a KISS (Keep It Simple, Stupid Keep it Short and Simple) solution is all that's needed.

Design patterns are great for making a system flexible, but at the cost of making the implementation more complex. This is only a worthwhile trade off when the flexibility will provide a practical advantage.

Answer 9

I just wanted to add another comment after seeing some of the "All patterns are bad" answers.

If you are a half decent programmer working on moderately challenging problems, nearly all the patters should have presented themselves to you at one point or another as the "Obvious" solution to a problem.

The only real point of the Design Patterns book was to put names to what we all do every day so we could communicate better.

I suppose if you are a newish programmer, it would be very helpful to read through them so that the day you need one you don't have to figure it out on your own, it's already in your toolbox, but in general--any of these could be figured out by a Gang of One (anyOne!).

If there are any of these you didn't already know, you probably just never needed it.

It is pretty useful to put names to the patters and formalize them a little though, I'm not complaining about the book at all. If you don't see the occasional need for nearly all of the patterns here, you just aren't working on very hard code (or you duplicate code a lot, something I consider THE cardinal sin).

Answer 10

The big one I see is the singleton pattern where not enough care and dilligence is applied as to how and when a singleton's destructor should be called.

For such a ubiquitous pattern there is hardly any discussion about the proper process to decide when a singleton must die.

Just my 0.02.

cheers,

Rob

Answer 11

The observer pattern is pretty useless in C# because it has events.

Answer 12

First, "it depends" on the language - some structures in some languages lessen the need for certain design patterns.

Second, part of the template for the concept of a Design Pattern from the start has included sections for "Applicability" and "Consequences" - ignore these at your own risk. "Knowing" a pattern doesn't just mean you know how to code it in the language of your choice - it also means knowing when to use it, and what drawbacks using it may entail.

Answer 13

You can't have straight answer for this question. It's mostly subjective and depends on application requirement.

1. Most of the people quoted that Singleton_pattern is bad but it's not bad for every user and project. For my project requirement, it serves the purpose. I need one ConnectionManager to handle session management between Client and Application and Singleton does the job brilliantly.

2. You have given a third party jar with good documentation. The jar contains inheritance hierarchy. Now you have to add a operation on each child. Since you don't have source code, you can't do it. Now you can benefit by using Visitor_pattern. But if you have source code with you, you may not use Visitor pattern at all. You can simple add new operation in parent and each child. Lack of source code does not imply that I am misusingVisitor pattern.

3. I want to limit communication between various objects. I will go-ahead with implement Mediator_pattern for object communication. I want to limit the client exposure to my system to hide the complexity. I will go-ahead with Facade_pattern. That does not mean that I am Misusing these patterns. This same example can be extended to other patterns like Proxy_pattern etc.

Answer 14

Only use a pattern when its called for. You can't predict the future, so while you might put a pattern in to make the design flexible, what happens when the product takes a different direction and your pattern becomes the very thing that's keeping you from implementing what your users want?

Keep designs as simple as possible to start with. When you know more about how your design needs to change, use the appropriate pattern and not before.

Answer 15

REPOSITORY PATTERN

Most people start using this pattern right after reading the Domain Driven Design book by Eric Evans.

How many folks here have seen repositories constructed like data access objects?