This post is meant as a follow-up of my previous series about architectural design.
All design patterns can be good or bad, depending on where and why they are used. Sometimes I see code using patterns which I would never have used myself because they just make everything harder. In that case, I consider them to be anti-patterns; because they are used to solve something that is designed in the wrong way.
This is only my opinion, I’m sure you might think differently. They are based on my experience with business applications.
Service locator
This one is described as an anti-pattern all over the internet. Although I think that service locator is not an anti-pattern in itself, using it in the wrong way for the wrong reasons makes it a very bad practice.
If you use dependency injection (and you probably should), constructor injection is the preferred way to inject dependencies into your objects. The reason is simple: service locator hides your dependencies, which means that you don’t know them when a class gets instantiated and errors might be thrown at runtime. Also maintenance becomes more difficult, because it’s hard to see your dependencies.
Now, in the case of exceptional occasions, service locator might be the only solution to have your dependencies injected. For example, if you can’t use a constructor overload (an attribute, for example); or some legacy component that is not designed for inversion of control, service locator can come to the rescue.
My advice is to always prefer constructor injection; and if you really really really don’t have another choice, use service locator.
Static facades
As defined, “a facade is an object that provides a simplified interface to a larger body of code”. However, static facades are evil, they are a sign of procedural code and often go hand in hand with too many global variables.
In fact, I don’t see any reason to make them static, because it just kills the flexibility of your system. It breaks the dependency injection container, they are very limited in usage and they simply make it difficult to test (it’s hard to mock them out). It does not fit in a system designed for scalability. The only exception I think of are some cross-cutting concerns that are used broadly across the system, like the built-in System.Math class in the .NET framework that contains mathematical utility methods; or when you really need singleton behavior (on rare occasions).
My advice: if you have static facades (or static methods in general), you might want to rethink the design.
Layers with identical entities and auto mapping
In a layered design you have a number of different entity types:
- Data entities: the entities that represent the database objects, defined in the data access components of the infrastructure layer.
- Domain entities: entities that contain domain data and logic in the domain layer.
- Data Transfer Objects: the entities that are exposed by services and used by the clients.
Sometimes I see three layered systems where data entities, domain entities and data transfer objects map one to one; in other words, they are identical. I think this is a bad practice. In fact, you are exposing your data model to your clients, and in fact, because these entities are all the same, why do you need all the layers in the first place? Often this mapping is auto generated by T4 templates, or a framework like object mapper is used to automatically map them; to me this is a sign that something is terribly wrong.
So what would be a better approach?
- The data entities map one to one to the database tables.
- The data entities map to the domain entities, but domain entities might be designed differently, so that they really represent your domain (and not the way they are stored in the database).
- The domain entities map to the data transfer objects, but data transfer objects might look completely different as they are designed in the way that only data is exposed that is really needed by the clients. Your clients don’t need to know about your domain model anyway.
My advice is to design your domain model independent of your database tables, and create data transfer objects that are tailored towards your clients.
Note: if you use Entity Framework code first, data entities are obsolete, because you map database tables directly to your domain entities.
Anemic domain model
This is a well known anti-pattern. Simply put: if your domain entities only contain data and no domain logic, then there is a problem of responsibilities. Probably the domain logic is in another place where it simply does not belong. I’m not going to explain this in detail because there’s more than enough literature to be found on the internet, but unfortunately it still is a common practice even in cases where a fully fledged domain model should be in place.
My advice: if you call your entities ‘domain entities’, it means that you probably need a domain and as such, put domain logic where it belongs: in your domain entities.
Lazy loading
“Lazy Loading is a programming practice in which you only load or initialize an object when you first need it when using an object relational mapper”. I think it should be avoided, because a service should expose clearly defined business operations that make clear in advance what you can expect from them to return (instead of CRUDy operations, unless you really have no business logic). If you do that, there are no surprises and less potential performance problems and side effects; because it’s the service that takes care of the querying. It’s more like controlled eager loading. Of course, you have to design your services in such a smart way that you don’t return too much information, which is the reason you want specialized data transfer objects and clear business operations with limited scope.
My advice: whatever ORM you are using, turn off lazy loading by default.
Hungarian notation
I admit that many many years ago I once named my variables like tbxFirstName, lblAddress and so on. After a while I really started to hate it, and I stopped doing it. It began to look as I was obfuscating my code! Indeed, I consider this to be an anti-pattern because:
- It complicates naming, because you have to agree on prefixes and eventually you will end up with hundreds of prefixes that are impossible to remember; sometimes more than 15 random characters. Of course, variable names will also become longer. Eventually, it becomes harder to understand.
- Sometimes types can change, and in that case your variable names have to change too.
- The purpose is to indicate type of variables, but with intellisense you can also quickly see that.
- See wikipedia for the full list of reasons.
My advice: name your variables consequently so that they show their intent (make them descriptive), not their type.
Other?
If I think of other patterns that are used in a bad ways, I will add them. If you have other examples of bad practices, feel free to share them!
Dunatis 