I have been using Entity Framework 6, AutoMapper, ASP.NET Web Api and hit a few problems along the way, each time it was recommended that I could change my code from Inheritance to use composition design instead. After the last problem, and since the software is in early stages, I thought I would try the composition method and see how it panned out. Now I know that composition can mean that you write out methods multiple times, unlike inheritance, but I understand the tradeoffs and I think it's fine, but I wonder if I have missed something here.
I have an EntityBaseService which was trying to deal with an object sometimes needing validation. Using inheritance I would have:
public interface IEntityBaseService<TEntity>
where TEntity : class, IEntityBase
{
Task<ICollection<TEntity>> GetAllAsync();
Task<TEntity> GetAsync(long id);
Task<ValidationResult> ValidateAsync(TEntity t);
Task<int> AddAsync(TEntity t);
Task<int> UpdateAsync(TEntity updated);
Task<int> DeleteAsync(TEntity t);
}
public abstract class EntityBaseService<T> : IEntityBaseService<T>
where T : class, IEntityBase
{
protected IDataContext _context;
protected IValidator<T> _validator = null;
protected EntityBaseService(IDataContext context)
{
_context = context;
}
protected EntityBaseService(IDataContext context, IValidator<T> validator)
: this(context)
{
_validator = validator;
}
public virtual async Task<ICollection<T>> GetAllAsync()
{
return await _context.Set<T>().ToListAsync();
}
public virtual Task<T> GetAsync(long id)
{
return _context.Set<T>().Where(e => e.Id == id).FirstOrDefaultAsync();
}
public virtual Task<ValidationResult> ValidateAsync(T t)
{
if (_validator == null) throw new MissingFieldException("Validator does not exist for class " + t.GetType().ToString() + ". override method if no validation needed");
return _validator.ValidateAsync(t);
}
public virtual async Task<int> AddAsync(T t)
{
var results = await ValidateAsync(t);
if (!results.IsValid) {
throw new ValidationException(results.Errors);
}
if (_context.GetState(t) == EntityState.Detached)
{
_context.Set<T>().Add(t);
_context.SetState(t, EntityState.Added);
}
return await _context.SaveChangesAsync();
}
public virtual async Task<int> UpdateAsync(T updated)
{
var results = await ValidateAsync(updated);
if (!results.IsValid)
{
throw new ValidationException(results.Errors);
}
if (_context.GetState(updated) == EntityState.Detached)
{
_context.SetState(updated, EntityState.Modified);
}
return await _context.SaveChangesAsync();
}
public virtual Task<int> DeleteAsync(T t)
{
_context.SetState(t, EntityState.Deleted);
return _context.SaveChangesAsync();
}
}
And then another class, would inherit from this service, like so, with its own interface:
public interface IAttachmentService : IEntityBaseService<Attachment>
{
Task<AttachmentData> GetDataAsync(long id);
}
public class AttachmentService : EntityBaseService<Attachment>, IAttachmentService
{
public AttachmentService(IDataContext context, AttachmentValidator validator) : base(context, validator) { }
public async Task<AttachmentData> GetDataAsync(long id)
{
return await _context.Attachments
.Where(e => e.Id == id)
.Select(e => e.Data)
.FirstOrDefaultAsync();
}
}
As I said, a lot of methods I were overwriting (unlike this example). Trying composition, I have easily separated the logic of requiring validation and not, since I have now created a ValidationService, which can be added to an object that needs it:
public interface IEntityBaseService<T> where T : class, IEntityBase
{
IDataContext Context { get; }
Task<ICollection<T>> GetAllAsync();
Task<T> GetAsync(long id);
Task<int> AddAsync(T entity);
Task<int> UpdateAsync(T entity);
Task<int> DeleteAsync(T entity);
}
public class EntityBaseService<T> where T : class, IEntityBase
{
protected IDataContext _context;
public IDataContext Context
{
get
{
return _context;
}
}
protected EntityBaseService(IDataContext context)
{
_context = context;
}
public virtual async Task<ICollection<T>> GetAllAsync()
{
return await _context.Set<T>().ToListAsync();
}
public virtual Task<T> GetAsync(long id)
{
return _context.Set<T>().Where(e => e.Id == id).FirstOrDefaultAsync();
}
public virtual async Task<int> AddAsync(T entity)
{
if (_context.GetState(entity) == EntityState.Detached)
{
_context.Set<T>().Add(entity);
_context.SetState(entity, EntityState.Added);
}
return await _context.SaveChangesAsync();
}
public virtual async Task<int> UpdateAsync(T entity)
{
if (_context.GetState(entity) == EntityState.Detached)
{
_context.SetState(entity, EntityState.Modified);
}
return await _context.SaveChangesAsync();
}
public virtual Task<int> DeleteAsync(T entity)
{
_context.SetState(entity, EntityState.Deleted);
return _context.SaveChangesAsync();
}
}
Validation Service, now separated:
public interface IValidationService<T> where T : class, IEntityBase
{
Task<ValidationResult> ValidateAsync(T entity);
Task<int> AddAsync(T entity);
Task<int> UpdateAsync(T entity);
}
public class ValidationService<T> where T : class, IEntityBase
{
private EntityBaseService<T> _service;
private IValidator<T> _validator = null;
public ValidationService(EntityBaseService<T> service, IValidator<T> validator)
{
_service = service;
_validator = validator;
}
public Task<ValidationResult> ValidateAsync(T t)
{
if (_validator == null) throw new MissingFieldException("Validator does not exist for class " + t.GetType().ToString() + ". override method if no validation needed");
return _validator.ValidateAsync(t);
}
public virtual async Task<int> AddAsync(T t)
{
var results = await ValidateAsync(t);
if (!results.IsValid)
{
throw new ValidationException(results.Errors);
}
return await _service.AddAsync(t);
}
public virtual async Task<int> UpdateAsync(T updated)
{
var results = await ValidateAsync(updated);
if (!results.IsValid)
{
throw new ValidationException(results.Errors);
}
return await _service.UpdateAsync(updated);
}
}
Now a service that "CAN" implement the methods I need, with the logic I require for that object can look like this:
public interface IAttachmentService
{
Task<Attachment> GetAsync(long id);
Task<int> CustomAsync(long id);
Task<int> AddAsync(Attachment entity);
Task<int> UpdateAsync(Attachment entity);
Task<int> DeleteAsync(Attachment entity);
}
public class AttachmentService : IAttachmentService
{
private IEntityBaseService<Attachment> _service;
private IValidationService<Attachment> _validation;
public AttachmentService(IEntityBaseService<Attachment> service, IValidationService<Attachment> validation) {
_service = service;
_validation = validation;
}
public Task<Attachment> GetAsync(long id)
{
return _service.GetAsync(id);
}
public async Task<int> CustomAsync(long id)
{
// custom code
return _service.Attachments.Where(e => e.Number > id).FirstOrDefaultAsync();
}
public Task<int> AddAsync(Attachment entity)
{
return _validation.AddAsync(entity);
}
public Task<int> UpdateAsync(Attachment entity)
{
return _validation.UpdateAsync(entity);
}
public Task<int> DeleteAsync(Attachment entity)
{
return _service.DeleteAsync(entity);
}
}
Now is this right? I believe it is, but the inheritance part of my brain is saying that I need to somehow not have to create GetAsync, AddAsync, UpdateAsync etc in each object, but without doing so means that I cannot customize a function easily, without writing an override. Plus this all seems easily testable now, as I can test each service independently using a testing framework.
AttachmentServiceis wrapping all the method calls from two classes that have different goals. Your class is composed of parts, and those parts do what they need to do. You can make those into public properties (or{ get; private set; }if you prefer) and remove all the wrapping methods. \$\endgroup\$