Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

This is my architecture to EF4 using repository pattern and unit of work pattern with POCO. I believe I made some mistakes. Here is the architecture:

I have a solution with 5 projects:

  • MyApp.Common
  • MyApp.Data.EF4
  • MyApp.Domain.Company
  • MyApp.Domain.Transmission
  • MyApp.Web In addition, I am using StructureMap in this application in order to inject the data layer.

Now I will describe what each project contains - with source:

MyApp.Common - contains common classes and interfaces
Repository interface:

public interface IRepository<T> where T : class
{
    IQueryable<T> GetQuery(IEnumerable<Expression<Func<T, object>>> includes);

    IPaged<T> GetQuery(IQueryable<T> query,
        Func<IQueryable<T>, IOrderedQueryable<T>> orderBy, int pageNumber, int pageSize);

    IPaged<T> GetQuery(IEnumerable<T> query,
        Func<IEnumerable<T>, IOrderedEnumerable<T>> orderBy, int pageNumber, int pageSize);

    IEnumerable<T> GetObjectStateManagerChanges();

    void Insert(T entity);
    void MarkModified(T entity);
    void Delete(T entity);
    void Attach(T entity);
    void Detach(T entity);
    T GetOriginalEntity(Func<T, bool> predicate);
}

Unit of work interface:

public interface IUnitOfWork : IDisposable
{
    int Commit();
}

Unit of work factory interface:

public interface IUnitOfWorkFactory
{
    IUnitOfWork Create();
}

Unit of work:

  public static class UnitOfWork
    {
        private const string HTTPCONTEXTKEY = "MyApp.Common.HttpContext.Key";

        private static IUnitOfWorkFactory _unitOfWorkFactory;
        private static readonly Hashtable _threads = new Hashtable();

        public static void Commit()
        {
            IUnitOfWork unitOfWork = GetUnitOfWork();

            if (unitOfWork != null)
            {
                unitOfWork.Commit();
            }
        }

        public static IUnitOfWork Current
        {
            get
            {
                IUnitOfWork unitOfWork = GetUnitOfWork();

                if (unitOfWork == null)
                {
                    _unitOfWorkFactory = ObjectFactory.GetInstance<IUnitOfWorkFactory>();
                    unitOfWork = _unitOfWorkFactory.Create();
                    SaveUnitOfWork(unitOfWork);
                }

                return unitOfWork;
            }
        }

        public static void Dispose()
        {
            IUnitOfWork unitOfWork = GetUnitOfWork();

            if (unitOfWork != null)
            {
                unitOfWork.Dispose();
            }
        }

        private static IUnitOfWork GetUnitOfWork()
        {
            if (HttpContext.Current != null)
            {
                if (HttpContext.Current.Items.Contains(HTTPCONTEXTKEY))
                {
                    return (IUnitOfWork)HttpContext.Current.Items[HTTPCONTEXTKEY];
                }

                return null;
            }
            else
            {
                Thread thread = Thread.CurrentThread;
                if (string.IsNullOrEmpty(thread.Name))
                {
                    thread.Name = Guid.NewGuid().ToString();
                    return null;
                }
                else
                {
                    lock (_threads.SyncRoot)
                    {
                        return (IUnitOfWork)_threads[Thread.CurrentThread.Name];
                    }
                }
            }
        }

        private static void SaveUnitOfWork(IUnitOfWork unitOfWork)
        {
            if (HttpContext.Current != null)
            {
                HttpContext.Current.Items[HTTPCONTEXTKEY] = unitOfWork;
            }
            else
            {
                lock (_threads.SyncRoot)
                {
                    _threads[Thread.CurrentThread.Name] = unitOfWork;
                }
            }
        }
    }

Base repository for all entities:

public abstract class BaseRepository<T> where T : class
{
    protected IRepository<T> Repository { get; set; }

    public IEnumerable<Expression<Func<T, object>>> Includes { private get; set; }


    public BaseRepository()
    {
        Repository = ObjectFactory.GetInstance<IRepository<T>>();
        Includes = null;
    }

    protected virtual IQueryable<T> GetQuery()
    {
        return Repository.GetQuery(Includes).AsEnumerable().AsQueryable();
    }

    protected virtual IQueryable<T> GetQuery(params Expression<Func<T, object>>[] includes)
    {
        return Repository.GetQuery(includes);
    }

    protected virtual IPaged<T> GetQuery(IEnumerable<T> query,
        Func<IEnumerable<T>, IOrderedEnumerable<T>> orderBy, int pageNumber, int pageSize)
    {
        return Repository.GetQuery(query, orderBy, pageNumber, pageSize);
    }


    protected virtual IPaged<T> GetQuery(IQueryable<T> query,
        Func<IQueryable<T>, IOrderedQueryable<T>> orderBy, int pageNumber, int pageSize)
    {
        return Repository.GetQuery(query, orderBy, pageNumber, pageSize);
    }

    protected virtual IEnumerable<T> GetObjectStateManagerChanges()
    {
        return Repository.GetObjectStateManagerChanges();
    }

    protected virtual void Insert(T entity)
    {
        Repository.Insert(entity);
    }

    protected virtual void MarkModified(T entity)
    {
        Repository.MarkModified(entity);
    }

    protected virtual void Delete(T entity)
    {
            Repository.Delete(entity);
    }

    protected virtual void Attach(T entity)
    {
        Repository.Attach(entity);
    }

    protected virtual void Detach(T entity)
    {
        Repository.Detach(entity);
    }

    protected virtual T GetOriginalEntity(Func<T, bool> predicate)
    {
        return Repository.GetOriginalEntity(predicate);
    }
}

MyApp.Data.EF4 - Hold reference of MyApp.Common and implements all interfaces using the EF way. It also includes the MyAppModel.edmx that hold model for all the entities in the system. MyAppModel.edmx has Code Generation Strategy set to Default. The entity container name called: MyAppEntities.

Repository implementation:

public class Repository<T> : IRepository<T> where T : class
{
    ObjectContext _context;
    IObjectSet<T> _objectSet;

    protected ObjectContext Context
    {
        get
        {
            if (_context == null)
            {
                _context = GetCurrentUnitOfWork<EFUnitOfWork>().Context;
            }

            return _context;
        }
    }

    protected IObjectSet<T> ObjectSet
    {
        get
        {
            if (_objectSet == null)
            {
                _objectSet = this.Context.CreateObjectSet<T>();
            }

            return _objectSet;
        }
    }

    public TUnitOfWork GetCurrentUnitOfWork<TUnitOfWork>() where TUnitOfWork : IUnitOfWork
    {
        return (TUnitOfWork)UnitOfWork.Current;
    }

    public virtual IQueryable<T> GetQuery(IEnumerable<Expression<Func<T, object>>> includes)
    {
        return ObjectSet.IncludeMultiple(includes);
    }

    public virtual IPaged<T> GetQuery(IQueryable<T> query,
        Func<IQueryable<T>, IOrderedQueryable<T>> orderBy, int pageNumber, int pageSize)
    {
        if (orderBy != null)
        {
            query = orderBy(query);
        }

        IPaged<T> page = new Paged<T>(query, pageNumber, pageSize);

        return page;
    }

    public virtual IPaged<T> GetQuery(IEnumerable<T> query,
        Func<IEnumerable<T>, IOrderedEnumerable<T>> orderBy, int pageNumber, int pageSize)
    {
        if (orderBy != null)
        {
            query = orderBy(query);
        }

        IPaged<T> page = new Paged<T>(query, pageNumber, pageSize);

        return page;
    }

    public virtual IEnumerable<T> GetObjectStateManagerChanges()
    {
        return this.Context.ObjectStateManager.
            GetObjectStateEntries(EntityState.Added | EntityState.Modified).
            Select(e => e.Entity).
            OfType<T>();
    }

    public virtual void Insert(T entity)
    {
        this.ObjectSet.AddObject(entity);
    }

    public virtual void Delete(T entity)
    {
        this.ObjectSet.DeleteObject(entity);
    }

    public virtual void MarkModified(T entity)
    {
        this.Context.ObjectStateManager.ChangeObjectState(entity, EntityState.Modified);
    }

    public virtual void Attach(T entity)
    {
        ObjectStateEntry entry = null;
        if (this.Context.ObjectStateManager.TryGetObjectStateEntry(entity, out entry) == false)
        {
            this.ObjectSet.Attach(entity);
        }
    }

    public virtual void Detach(T entity)
    {
        ObjectStateEntry entry = null;
        if (this.Context.ObjectStateManager.TryGetObjectStateEntry(entity, out entry) == true)
        {
            this.ObjectSet.Detach(entity);
        }
    }

    public virtual T GetOriginalEntity(Func<T, bool> predicate)
    {
        T originalEntity = null;
        EFUnitOfWorkFactory factory = new EFUnitOfWorkFactory();
        using (EFUnitOfWork uow = (EFUnitOfWork)factory.Create())
        {
            originalEntity = uow.Context.CreateObjectSet<T>().Single(predicate);
        }
        return originalEntity;
    }
}

Extension that used in the repository implementation:

public static class Extensions
    {
        public static IQueryable<T> IncludeMultiple<T>(this IQueryable<T> query,
            IEnumerable<Expression<Func<T, object>>> includes)
            where T : class
        {
            if (includes != null)
            {
                query = includes.Aggregate(query,
                          (current, include) => current.Include(include));
            }

            return query;
        }
    }

Implementation for IUnitOfWork:

public class EFUnitOfWork : IUnitOfWork, IDisposable
{
    public ObjectContext Context { get; private set; }
    public int Id { get; private set; }

    public EFUnitOfWork(ObjectContext context, int id)
    {
        Id = id;
        Context = context;
        Context.ContextOptions.LazyLoadingEnabled = false;
    }

    public int Commit()
    {
        return Context.SaveChanges();
    }

    public void Dispose()
    {
        if (Context != null)
        {
            Context.Dispose();
            Context = null;
        }

        GC.SuppressFinalize(this);
    }
}

Implementation of IUnitOfWorkFactory:

public class EFUnitOfWorkFactory : IUnitOfWorkFactory
    {
        private static int Counter = 0;
        private static Func<ObjectContext> _objectContextDelegate;
        private static readonly Object _lockObject = new object();

        public static void SetObjectContext(Func<ObjectContext> objectContextDelegate)
        {
            _objectContextDelegate = objectContextDelegate;
        }

        public IUnitOfWork Create()
        {
            ObjectContext context;

            lock (_lockObject)
            {
                Counter++;
                context = _objectContextDelegate();
            }

            return new EFUnitOfWork(context, Counter);
        }
    }

MyApp.Domain.Company and MyApp.Domain.Transmission - Contains POCO objects that should hold the query results and be the business logic of the solution. Each project reflects other and different need of the system. But, entities from MyApp.Domain.Company have relationships in the DB and in the edmx with MyApp.Domain.Transmission entities. For example: Vehicle that exists in MyApp.Domain.Company references by VehicleTransmission that exists in MyApp.Domain.Transmission. In addition, VehicleTransmission holds reference of Vehicle and therefore MyApp.Domain.Transmission contains reference of MyApp.Domain.Company. Both projects hold reference to MyApp.Common. It is importent to remember that T_TBL_VEHICLE and T_TBL_VEHICLE_TRANSMISSION (and the connections between them) represented in the same edmx file. Each cs file under those projects contains the POCO entity and a repository manager.
Vehicle.cs (from MyApp.Domain.Company)

public class Vehicle
    {
        public virtual long Id { get; set; }
        public virtual string VehicleNumber { get; set; }

        public virtual DateTime? PurchaseDate { get; set; }
        public virtual string InsuranceAgency { get; set; }
        public virtual DateTime? InsuranceValidity { get; set; }
        public virtual string Comments { get; set; }
        public virtual IList<Worker> Workers { get; set; }        

        public string GetPhoneNumber()
        {
            string phoneNumber = null;
            if (PhoneId.HasValue)
            {
                phoneNumber = Phone.PhoneNumber1;
                if (string.IsNullOrEmpty(phoneNumber))
                {
                    phoneNumber = Phone.PhoneNumber2;
                }
            }
            return phoneNumber;
        }

        public string GetManufacturerName()
        {
            string name = null;
            if (ManufacturerId.HasValue)
            {
                name = Manufacturer.Name;
            }
            return name;
        }
    }

    public class VehicleRepository : BaseRepository<Vehicle>
    {
        private bool IsCounterExists(Guid companyId, long counter)
        {
            return GetQuery().
                Where(x => x.CompanyId == companyId).
                Where(x => x.IsDeleted == false).
                AsEnumerable().
                Any(x => x.Code.ToNullable<long>() == counter);
        }

        public void Attach(Vehicle entity)
        {
            base.Attach(entity);
        }

        public void MarkModified(Vehicle entity)
        {
            base.MarkModified(entity);
        }

        public void Delete(Vehicle entity)
        {
            base.Delete(entity);
        }

        public void Insert(Vehicle entity)
        {
            if (entity.VehicleNumber != null) { entity.VehicleNumber.Trim(); }
            if (entity.VehicleNumber == null || entity.VehicleNumber == string.Empty)
            {
                throw new MissingFieldException(
                    CompanyExceptionMessage.MissingFieldException_Vehicle_Number.Message);
            }
            if (string.IsNullOrEmpty(entity.Code))
            {
                StaffCounterRepository rep = new StaffCounterRepository();
                entity.Code = rep.GetAndAdvanceCounter(entity.CompanyId,
                    StaffCounterEnum.VEHICLE,
                    counter => IsCounterExists(entity.CompanyId, counter)).ToString();
            }

            int equals = GetQuery().
                 Where(x => x.CompanyId == entity.CompanyId).
                 Where(x => x.IsDeleted == false).
                 Where(x =>
                     (x.Code != null && x.Code == entity.Code) ||
                     (x.VehicleNumber == entity.VehicleNumber)
                     ).Count();
            if (equals > 0)
            {
                throw new ExistsNameOrCodeException(
                    CompanyExceptionMessage.ExistsNumberOrCodeException_Vehicle);
            }
            base.Insert(entity);
        }

        public Vehicle Get(Guid companyId, long vehicleId)
        {
            return GetQuery().Where(x => x.CompanyId == companyId).
                Single(x => x.Id == vehicleId);
        }

        public IQueryable<Vehicle> Get(Guid companyId)
        {
            return GetQuery().
                Where(x => x.CompanyId == companyId).
                Where(x => x.IsDeleted == false).
                OrderBy(x => x.VehicleNumber);
        }

        public Vehicle Get(Guid companyId, string code)
        {
            return GetQuery().
                Where(x => x.CompanyId == companyId).
                Where(x => x.IsDeleted == false).
                FirstOrDefault(x => x.Code == code);
        }

        public IQueryable<Vehicle> Search(Guid companyId, string vehicleNumber)
        {
            var query = GetQuery().
                Where(x => x.CompanyId == companyId).
                Where(x => x.IsDeleted == false);
            if (vehicleNumber != null)
            {
                query = query.Where(x => x.VehicleNumber.Contains(vehicleNumber));
            }
            return query.OrderBy(x => x.VehicleNumber);
        }

        public IEnumerable<Vehicle> Get(Guid companyId, bool? freeVehicles,
            bool includeContractorVehicles)
        {
            IEnumerable<Vehicle> query = GetQuery().
                Where(x => x.CompanyId == companyId).
                Where(x => x.IsDeleted == false);

            if (freeVehicles == true)
            {
                query = query.Where(x => x.Workers.Count == 0);
            }
            else if (freeVehicles == false)
            {
                query = query.Where(x => x.Workers.Count > 0);
            }
            query = query.AsEnumerable();

            if (includeContractorVehicles == true)
            {
                WorkerRepository rep = new WorkerRepository();
                IEnumerable<Vehicle> contractorsVehicles = rep.Get(companyId).
                    Where(x => x.ContractorVehicleNumber != null &&
                        x.ContractorVehicleNumber != string.Empty).
                    AsEnumerable().Select(x => new Vehicle()
                    {
                        VehicleNumber = x.ContractorVehicleNumber
                    });
                query = query.Union(contractorsVehicles);
            }

            return query;
        }        

        public IQueryable<Vehicle> Get(Guid companyId, VehicleStatusEnum? status,
            string code, string vehicleNumber)
        {
            var query = GetQuery().
                Where(x => x.CompanyId == companyId).
                Where(x => x.IsDeleted == false);

            if (status.HasValue)
            {
                long? statusId = VehicleStatusWrapper.GetId<VehicleStatusWrapper>(status.Value);
                query = query.Where(x => x.StatusId == statusId);
            }
            if (!string.IsNullOrEmpty(code))
            {
                query = query.Where(x => x.Code.Contains(code));
            }
            if (!string.IsNullOrEmpty(vehicleNumber))
            {
                query = query.Where(x => x.VehicleNumber.Contains(vehicleNumber));
            }
            return query;
        }
    }

VehicleTransmission.cs (from MyApp.Domain.Transmission)

public class VehicleTransmission
    {
        public long? VehicleId { get; set; }
        public string VehicleNumber { get; set; }
        public long? SubcontractorId { get; set; }
        public bool WorkerOnHold { get; set; }

        public Vehicle Vehicle { get; set; }

        public void SetVehicleId(long? vehicleId)
        {
            VehicleId = vehicleId;
        }

        //Possibility to set vehicle that is not exists on the db
        public void SetVehicleNumber(string vehicleNumber)
        {
            VehicleId = null;
            VehicleNumber = vehicleNumber;
        }
    }

    public class VehicleTransmissionRepository : BaseRepository<VehicleTransmission>
    {
        public IQueryable<VehicleTransmission> Get()
        {
            return GetQuery();
        }
}

MyApp.Web - Contains the web application. Holds references to all other projects.
In the global.asax, in Application_Start I configure StractureMap:

    ObjectFactory.Configure(x =>
    {
        x.For<IUnitOfWorkFactory>().Use<EFUnitOfWorkFactory>();
        x.For(typeof(IRepository<>)).Use(typeof(Repository<>));
        x.For(typeof(IEnumRepository<,>)).Use(typeof(EnumRepository<,>));
    });
    EFUnitOfWorkFactory.SetObjectContext(() => new MyAppEntities());

In addition, in Application_EndRequest I do:

UnitOfWork.Dispose();

I also have web services where there I use the repositories and call UnitOfWork.Commit() when needed.

So far, this is my architecture. I really want to know if I made things right and where are my pitfalls.

What do you think?

share|improve this question
    
Is there any way you can shorten the code snippets and provide links to the larger examples? –  IAbstract Oct 25 '11 at 1:28
    
@IAbstract: What do you mean by "links to the larger examples"? –  Naor Oct 26 '11 at 8:52
    
@Naor If you can just pad every character with additional whitespace and for every line add 5 lines of comments. That should make the code larger. –  Jeremy Child Oct 27 '11 at 2:28
    
Well you can use UnitOfWorkScope to manage life cycle of UnitOfWork and take different UnitOfWorkScope such as WCFUnitOfWorkScope, WebUnitOfWorkScope. Their functionality are not applicable by dependency injection containers. –  user19946 Dec 2 '12 at 16:49
add comment

3 Answers

Without understanding too much of your scenario, here are a few pointers:

I'm not that familiar with Entity Framework, but a common interface for UoW usually have the following behaviors:

public interface UnitOfWork : IDisposable {
  bool IsInTransaction { get; }
  bool IsDirty { get; } // Same as your MarkModified()
  void BeginTransaction();
  void Commit();
  void Rollback();
}

Where is the error and logging handling? Below is an example of a simple wrapper you can use.

// Example call
ExceptionHandler.Try("Could not load Vechicle", () => _vehicleRepository.Insert(vehicle));

 // Example class
 public class ExceptionHandler
    public void Try(string errorMessage, Action func)
    {
        try
        {
           func();
        }
        catch (Exception e)
        {
           Handle(e, errorMessage); // handle and log exception
        }
    }

And change the name of the Common-project to something less common..;)

share|improve this answer
2  
+1 for the ExceptionHandler recommendation and snippet. –  IAbstract Oct 27 '11 at 13:38
add comment

This is actually very similar to my implementation...one major difference is that I didn't implement the life-cycle management for the UOW. I handle that with dependency injection (Ninject using Request Scope) which produces the same result.

Another major difference in my implementation is that I also have an ObjectContextResolver and an ObjectContextFactory. The Unit Of Work and the Repositories use the ObjectContextResolver to gain access to the current Context where the ObjectContextFactory is used by the ObjectContextResolver to create new instances of the Context when one does not exist. The ObjectContextFactory also uses an ObjectContextActivationInfo which is a wrapper for the connection string and settings, allowing me to easily vary the way the context is created.

I also have two types of repositories...I have an EntityRepository which is much like your implementation and allows me to query and work with specific entities. I also have a ContextRepository which exposes a GetSet<T> method so I can query any entity type in my model.

I also allow my Unit Of Work scope to be a bit more flexible and let the consuming application define the beginning and end of the unit of work. A typical usage looks like this...

using(var unitOfWork = this.UnitOfWorkFactory.NewUnitOfWork())
{
     try
     {
          var myVehicle = this.VehicleService.GetVehicleById(123);
          myVehicle.VehicleNumber = this.MyExternalWebService.GetNewVehicleNumber();
          myVehicle.DateModified = DateTime.Now();

          unitOfWork.Commit();
     }
     catch(Exception ex)
     {
          // The Web Service Call failed, don't save the changes
          unitOfWork.Rollback();
     }

}

This also allows separate Units Of Work to occur with a single Request which is important if some things need to get saved before other things can get created...this happens often when external services or legacy data stores are used in conjunction with EF.

Because I've wrapped my code in a using block, once execution is complete, it is automatically disposed.

I also have a Rollback() method on my UnitOfWork that allows changes to be reverted in cases where some code has failed and you don't want to save the changes.

One limitation of this architecture (both yours and mine) is the inability to perform operations using the UnitOfWork in parallel. I recently had a need to execute two EF queries through my framework in parallel and was not able to do so because the code essentially replaces the current UnitOfWork since my architecture is designed around a single Http Request.

I've been pondering ways to extend my current approach to allow that.

share|improve this answer
    
Update, I resolved the issue of using these units of work in parallel, but only in read only scenarios. –  ctorx Feb 11 '13 at 7:00
    
Very interesting. I like how it is used in the end. I wonder if you couldn't pass the UOW instance to the methods in the services somehow? That would be flexible enough to handle any scenario, though I admit it would be potentially a lot more clumsy code-wise, since it would need to be passed to every repository method. –  julealgon Feb 19 at 15:37
1  
I think passing the UOW to the service would defeat the purpose since the UOW scope can span multiple service calls. Assume you have a logical UOW that makes three calls to separate services. If you passed the UOW along, any one of those service methods would be able to Commit or Rollback the UOW. This would force you to continually check the state of the UOW so you'd know if the next service call should commence. I've been using the in app UOW scope for some time now, and I find it very simple, straight forward and reasonably flexible. –  ctorx Feb 24 at 17:48
add comment

Entity Framework IS the Unit of Work in the system so you don't have to reinvent the wheel. Only one you should create: a generic repository base:

interface IRepository<TEntity>
    where TEntity : class
{
    //Add<TEntity>, Remove<TEntity> ...
}

class /* don't have to be abstract */ BaseRepository<TObjectContext, TEntity> : IRepository<TEntity>
    where TObjectContext : ObjectContext
    where TEntity : class
{
    public RepositoryBase(TObjectContext context)
    {
        //... store context
    }

    //implementations
}

And then you can create specialized repositories.

share|improve this answer
    
It is indeed the unit of work, but if you want to allow control of the lifecycle of the unit of work you need to either create a layer on top of EF that is used by the application or you have to work with Contexts directly in your applications. –  ctorx Oct 18 '12 at 14:53
    
For lifecycle management use some dependency injection container. (For example Ninject can do it.) –  Peter Kiss Oct 18 '12 at 15:06
    
My implementation uses Ninject to manage the lifecycle of the some of the lower level components used to build the context, but not the context itself. This way, my app knows nothing of Contexts. It only knows about services which know about repositories which know about context. –  ctorx Oct 18 '12 at 19:04
    
@PeterKiss it is true EF is a UoW but, by default EF will let you do anything with its context, what happens when your objects dont support every method, let's say you dont want to be able to delete logs, for example. You could always do a generic service wrapper with virtual methods and override every not supported method with a NotSupportedException, or handle this in another way, but again, how do you hide your not supported methods? I believe, and this is in my honest and humble oppinion, where the combination of Repository pattern and uow shines. –  Esteban Sep 26 '13 at 3:16
add comment

protected by syb0rg Mar 1 at 23:48

Thank you for your interest in this question. Because it has attracted low-quality answers, posting an answer now requires 10 reputation on this site.

Would you like to answer one of these unanswered questions instead?

Not the answer you're looking for? Browse other questions tagged or ask your own question.