In software engineering, an anti-pattern (or antipattern) is a design pattern that appears obvious but is ineffective or far from optimal in practice.1 Or as I like to think of it, the type of code that makes you want to rip your eyes out, like in that movie Event Horizon…
Here’s a pattern that I had witnessed several times in a particular codebase, which I believe suffers from some design flaws:
FooListing is a class that retrieves and maintains a list of Foo objects
FooListing is a repository-type object that holds a list of
Foo objects, which it populates
from the database using its
populate() function. The
Foo object also has a
method, but this method instantiates a
FooListing, and uses the
FooListing populate function
with its key - so now the
FooListing contains the desired
Foo object. Our original
object now contains a reference to the
Foo object that it
wants to be - so it does what
anyone would do, steals its identity and hides the body.
Foo.populate() looks something like this:
Why this is bad
Firstly, apart from the questional morality, it
is duplicated code.
FooListing already has the capability to create and
Foo object from the database. Two locations for this code means
twice the maintenance if something changes, more possibility
of bugs, etc.
FooListing have become tightly coupled and
dependant on each other under this design - there is a cyclic dependancy which
is a code smell, and may cause headaches when writing unit tests.
There is also a waste of resoures creating the uneccesary
objects inside of
Foo.populate(), at least some of that could be avoided by
client code accessing instances of
It also doesn’t make sense that a data access object like
Foo should be concerned
with information about how it is created.
Foo should act more like a bean,
or an immutable class.
Another dangerous aspect of this design is the
implication that client code accessing
Foo cannot be certain that
been instantiated correctly. If the client code has a faulty key for
that does not exist in the database, when it creates new
Foo(key) there is
no way to know that
Foo.populate() has failed to find the correct value, and
instead they are left with a faulty
Foo instance which was not what they
The best solution for this isolated pattern is to
completely remove (or deprecate) the
Foo.populate() method, and replace
calls to it with
FooListing fails to find a
Foo, the client code should realise this when
them a null object. The client code can handle and recover from this case in
getFirstResult() function in
be beneficial if there are many cases where the code with otherwise be calling
We could also simplify the calling code so that retrieving a
result is a one-line operation - i.e.
populate() if the list
has not already been populated.
And the client code would only need to call: