Problem
I need some thoughts about an approach a colleague and I am are taking with Entity Framework. Basically, the entities are represented by contracts. These contracts hold a collection of business rules so when they are finally persisted we know they are valid. We can also check the validity from Entity Framework, but that is not as important. Since EF requires you not to have a parameterized constructor we do not pass the actual domain entity into EF, but rather the contract that represents it.
So how this works is, first a context is established. Next, we create a contract and then create the domain entity by passing it the contract. Inside the constructor of the domain entity a call to an IsValid method is made. This runs through all the business rules. If any one fails, the domain entity is considered invalid and an exception of some kind is thrown. If it is valid, we then add the contract to the context and persist the changes and pass back the domain entity. Here is some code that demonstrates the idea:
/// <summary>
/// Interface the all Contracts will impelement
/// </summary>
public interface IEntityContract
{
bool IsValid();
}
/// <summary>
/// Business Rule Loigc
/// </summary>
public class BusinessRule<TEntity> where TEntity: IEntityContract
{
//Logic to handle business rules
}
/// <summary>
/// Abstract class that represents a Base Contract
/// </summary>
/// <typeparam name="TEntity"></typeparam>
public abstract class EntityContract<TEntity>: IEntityContract where TEntity: IEntityContract
{
private readonly List<BusinessRule<TEntity>> _businessRules = new List<BusinessRule<TEntity>>();
public bool IsValid()
{
var entityIsValid = true;
foreach (var businessRule in _businessRules)
{
//IF any rule fails the entityIsValid = fale
}
return entityIsValid;
}
protected void RegisterBusinessRule(BusinessRule<TEntity> rule)
{
_businessRules.Add(rule);
}
}
public interface IUserContract: IEntityContract
{
string UserName { get; set; }
string Password { get; set; }
}
internal sealed class UserContract: EntityContract<IUserContract>, IUserContract
{
public UserContract()
{
this.RegisterBusinessRule(new BusinessRule<IUserContract>());
this.RegisterBusinessRule(new BusinessRule<IUserContract>());
}
public string UserName { get; set; }
public string Password { get; set; }
}
/// <summary>
/// Abstract class the represetn a base domain entity
/// </summary>
/// <typeparam name="TEntity"></typeparam>
public abstract class EntityBase<TEntity> where TEntity: IEntityContract
{
protected EntityBase(TEntity contract)
{
if(!contract.IsValid())
{
throw new Exception("Business Rule Violation");
}
}
public Guid Id { get; set; }
protected void BuildProperties()
{
//This method will iterate the TEntity properties from the contract and build the actual domain entity
}
}
/// <summary>
/// Class that represent a domain entity
/// </summary>
public class User: EntityBase<IUserContract>
{
public User(IUserContract contract) : base(contract)
{
this.BuildProperties();
}
}
//Domonstration of idea
public class TestClass
{
public void TestUserEnity()
{
//Using Entity Framework 4.1 we instantiate a DbContext
var dataContext = new SomeEntityFrameworkDbContext();
//Create the UserContract
var userContract = new UserContract {UserName = "someUserName@place.com", Password = "thePassword"};
try
{
//Create a new user and pass it the UserContract
var user = new User(userContract);
//If evertyhing goes ok, add the new eneity to the context
dataContext.Add(userContract);
//Persist the data
dataContext.SaveChanges();
}
catch (Exception)
{
//throw the excpetion and send back the failures
}
}
}
Solution
Considering Entity Framework does not like parameterized constructors, I think the only solution is when the entity is attached to the context or on SaveChanges we evaluate the the entity.
Perhaps instead of evaluating the entity directly in the context we do it in a class that employs the repository pattern. Now, this code is obviously not complete and we would want to leverage a Unit Of Work that abstracts the context, possibly. Here is the refactored code
public class User
{
public string Username { get; set; }
public string Password { get; set; }
}
public interface IRepository<TEntity> where TEntity : class
{
void Create(TEntity entity);
void Update(TEntity entity);
void Delete(TEntity entity);
}
public class Repository<TEntity> : IRepository<TEntity> where TEntity : class
{
private readonly IEntityContract<TEntity> _contract;
private readonly DbContext _context;
public Repository(IEntityContract<TEntity> contract, DbContext context)
{
_contract = contract;
_context = context;
}
public void Create(TEntity entity)
{
//beofre we try to create it, lets evaluate it
if (!_contract.IsValid(entity))
{
//possibly throw an exception or let the client know there was an issue.
}
//Now implement your create code.
_context.Add(entity);
_context.SaveChanges();
}
public void Update(TEntity entity)
{
//beofre we try to update it, lets evaluate it
if (!_contract.IsValid(entity))
{
//possibly throw an exception or let the client know there was an issue.
}
//Now implement your update code.
_context.Attach(entity);
_context.SaveChanges();
}
public void Delete(TEntity entity)
{
//delete code
}
}
/// <summary>
/// Interface the all Contracts will impelement
/// </summary>
public interface IEntityContract<TEntity> where TEntity: class
{
bool IsValid(TEntity entity);
void RegisterBusinessRule(BusinessRule<TEntity> rule);
IList<BusinessRule<TEntity>> Rules { get; set; }
}
/// <summary>
/// Business Rule Loigc
/// </summary>
public class BusinessRule<TEntity> where TEntity : class
{
//Logic to handle business rules
}
/// <summary>
/// Abstract class that represents a Base Contract
/// </summary>
/// <typeparam name="TEntity"></typeparam>
public abstract class EntityContract<TEntity> : IEntityContract<TEntity> where TEntity : class
{
protected EntityContract()
{
Rules = new List<BusinessRule<TEntity>>();
}
protected EntityContract(IList<BusinessRule<TEntity>> rules)
{
Rules = rules;
}
public virtual bool IsValid(TEntity entity)
{
var entityIsValid = true;
foreach (var businessRule in Rules)
{
//IF any rule fails the entityIsValid = false
}
return entityIsValid;
}
public IList<BusinessRule<TEntity>> Rules { get; set; }
public void RegisterBusinessRule(BusinessRule<TEntity> rule)
{
Rules.Add(rule);
}
}
public interface IUserContract : IEntityContract<User>
{
}
public class UserContract : EntityContract<User>, IUserContract
{
}
//Domonstration of idea
public class TestClass
{
public void TestUserEntity()
{
//Using Entity Framework 4.1 we instantiate a DbContext
var dataContext = new SomeEntityFrameworkDbContext();
var respository = new Repository<User>(new UserContract(), dataContext);
//Create the UserContract
var user = new User()
{
Password = "123456",
Username = "me@domain.com"
};
try
{
respository.Create(user);
}
catch (Exception ex)
{
//lets assume an excpetion gets thrown if the IsVlaid fails.
}
}
}
As you can see, now we take the responsibility of validating the entity away from the entity itself. Which, I feel is better to keep the entity as clean as possible and leave any logic out of the entity. This is how I have been feeling int he past few couple years.
your IUserContract inherits IEntityContract but doesn’t implement the bool IsValid(); method, I always assumed that if you inherit an interface that you have to implement everything inside that interface.
that is the only thing that I can see that looks out of place.
EDIT
as Retailcoder pointed out whatever inherits the last Interface would have to implement everything in all the interfaces in the chain. this means that inside the internal sealed class UserContract there should be an implementation of the bool IsValid(); method, which there isn’t.
The abstract class EntityBase is also missing the bool IsValid(); method
Merged answer
Having an Interface that only does one thing doesn’t seem right to me.
the way that you set up your User Class in your answer is much better than having an interface that forces you to create the User Credentials. I like that much better.
I also like the new IEntityContract<TEntity> as it forces more than just a IsValid Method. it looks like it has more of the logic that is needed for other things that will be forced in one way or another, reducing the amount of Interfaces and eventually code.
I like the new layout and structure that you have provided in your answer, it seems a lot clearer and more readable.
I know that this is a review of a review, but seeing as the OP is the one that posted the review that I am reviewing (still referring back to the original review) I think that this answer is still valid.