Is injecting DAO into entities a bad thing?

Question

So like most new .NET developers you start of passing DataSets everywhere and although things do the job it doesn't seem right.

The next progression is usually to create entity objects that extend a DAL base class so you have i.e.

public class User : UserDAL
{
    //User specific methods
}
public class UserDAL
{
  //Bunch of user specific properties
  public User Load(int Id)
  {
    //Some low level ADO calls
  }
  private User LoadFromDataSet(DataSet ds)
  {
     //Load entity properties from DataSet 
  }    
}

User extends UserDAL objects that has low level data access calls using ADO.NET.

From here you go on to learn that this implementation means your tied to a data access layer and you make use of a separate entity, data access object and DAO interface for mocking or to easily swap out the DAO if need be. i.e.

public UserDAO : IUserDAO
{
  //Data access Functions
}

With use of generics and reflection or a good ORM you can relieve some of the more common data access CRUD operations i.e.

Public UserDAO<User> : BaseDAO<User>, IUserDAO
{
  //BaseDAO deals with basic crud so more custom data access methods can go here 
}

So basically that's where I am currently at, aside from some other nice practices like using IoC to resolve the specific IUserDAO I want. But while I see the advantage to this structure I am also left feeling like I miss the old User.Load(1) method calls.

What I was wondering is would it be such a bad thing to inject my IUserDAO into the User entity and have that take care of the basic CRUD operations?

My understanding is that as a POCO the User entity would have no problems been passed over the wire and adding methods like Save(), Load() etc would have no relavence in the sense of a data transfer object.

But with that said my entities usually have lazy loaded collections which mean nothing in a DTO sense. Also, with WFP I believe can pick and chose which properties I want serializing, or at very least I could create a new UserDTO when I need to send it across the wire.

So basically, aside from that issue what are the other problems with making my User entity include DataAccess related methods? Also could someone clarify whether what I am talking about is referred to as an active record pattern or is this something else?

EDIT:

cristianlibardo noted:

As for potential drawbacks there is greater coupling to peristence code, resitence when following/updating associations, testability and querying.

There would be a greater some level of coupling but what I was thinking was somehting like the following:

public class User
{
   IUserDAO userDAO;
   public User()
   {
         userDAO = IoCContainer.Resolve<IUserDAO>;
   }
  public User(IUserDAO userDAO)
   {
         this.userDAO = userDAO;
   }
   //DAL methods
}

So the coupling should be minimal and as for testability, I don't see it as a problem as I can just inject a mock DAO into the entity.

Thanks to Brian Hasden, these are really good resources, but I guess I just wanted justification for something I was obviously going to do. Thanks for giving the said justification.

Answer 1

I came to the same conclusion. The load usually doesn't make sense in the entity because once you have an instance, you're either creating a new entity or you've already got a loaded entity. I've been using entities with Save (create and update) and Delete for years now without any issues. That being said, it's usually useful to still have a DAO to do other things, so you're not exactly combining the DAO and the entity. Most of the time for my entities the Save and Delete methods are just calling into the DAO Save and Delete methods.

BTW, I usually combine a dirty flag with the entity to know when properties have been changed so that you don't make unnecessary calls to save when the entity hasn't changed.

Typically, entities that don't really do anything but contain getters and setters for private members mean you're working with an anemic domain model. It's an anti-pattern that's fairly controversial.

You can find more information about it at:

http://www.martinfowler.com/bliki/AnemicDomainModel.html http://wrschneider.blogspot.com/2005/01/avoiding-anemic-domain-models-with.html http://www.dotnetjunkies.com/WebLog/richardslade/archive/2007/03/07/209401.aspx

Answer 2

Yes, what you're describing sounds just like active record (a database row made into an object with the logic to persist itself from/to the database). It's an effective technique, no doubt.

As for potential drawbacks there is greater coupling to peristence code, resitence when following/updating associations, testability and querying.

Answer 3

Keeping persistence out of your domain classes' inheritance models largely serves the goal of writing understandable and maintainable code.

Persistence is an orthogonal concern to what your class's real responsibilities are. I've noticed that the inherit-from-DAO approach arbitrarily divides the world into two categories of classes that, from the point of view of responsibilities and behaviors, are no different. Which side a class falls on will often change over time. You don't want that change to impact that class's API and interactions with other classes.

Example: Say that today, your Product class makes use of an IPriceCalculator, retrieved via IoC. Tomorrow, you decide you need to allow modification of the algorithms per-product, so your IPriceCalculator becomes a DAO. This could be a painful change, especially if your price calculators already had a base class supplying important functionality.

This example highlights another consideration: inheriting from a DAO class means your User class just lost the ability to inherit a Person class that may have useful behaviors. (In the .NET world, MarshalByRefObject is a common base class that people need to inherit which can conflict with inheritance-based DAO requirements.) With single inheritance, it's not ideal to inherit in order to gain functionality that's probably a separate concern.