Is it acceptable to use UnitOfWork from inside an entity

Question

I've been developing an ASP.NET MVC4 project from scratch by myself using separate project for my data access layer. There I am using Entity Framework 5 with Code First workflow and I've implemented I think a very standard Repository pattern plus Uint Of Work to separate the data layer from the business logic.

Everything was working as expected and I have already a lot of business logic, but I don't have too much experience (about 9 months total) and now a very experienced developer is joining to the project and suddenly I started to get this error:

The relationship between the two objects cannot be defined because they are attached to different ObjectContext objects.

The structure of my project is this -

Just a part from my UnitOfWork class :

 public class UnitOfWork : IDisposable
{
    private MyDbContext context = new MyDbContext ();

    private MenuRepository мenuRepository;
    public MenuRepository MenuRepository
    {
        get
        {
            if (this.мenuRepository == null)
                this.мenuRepository = new MenuRepository(context);
            return мenuRepository;
        }
    }

    //register other repositories...
}

And the my repositories have a standard look:

 public class MenuRepository : GenericRepository<Menu>, IMenuRepository
 {
     public MenuRepository(MyDbContext context) : base(context) { }
 }

I know that this is not enough to tell if the implementation is good enough or not but it was working for a few months without noticing any problems. Today I was implementing a logic for editing a menu with submenus :

Menu menu = unitOfWork.MenuRepository.GetById(model.MenuID);
    long menuItemId = menu.MenuItems[0].MenuItemID;
    if (menu != null)
    {
      menu.Name = model.MenuName;
      MenuItem menuItem = unitOfWork.MenuItemRepository.GetById(menuItemId);
      menuItem.Name = "Test";

      unitOfWork.MenuRepository.Update(menu);

      return View(model);
    }

This is all happening in my controller where I have this :

private UnitOfWork unitOfWork = new UnitOfWork();

So when I tried to execute this code I got the error from above for the ..attached to different ObjectContext objects..

The only change that I've noticed from before the code started to crash was this added from the other guy directly into the Menu entity :

//private DAL.UnitOfWork.UnitOfWork unitOfWork = new DAL.UnitOfWork.UnitOfWork();
    public Menu()
    {
        MenuItems = new List<MenuItem>();
    }

    public int MenuID { get; set; }
    //private int menuId;
    //public int MenuID {
    //    get
    //    {
    //        return menuId;
    //    }
    //    set
    //    {
    //        menuId = value;
    //        if (MenuItems.Count == 0)
    //        {
    //            MenuItems = unitOfWork.MenuItemRepository.GetBy(x => x.MenuID == menuId).ToList();
    //        }
    //    }
    //}

After I commented all the stuff above and returned it to public int MenuID { get; set; } without the additional logic, dropping the database and recreating it I was able to continue to use my Update logic from the controller without the error for the different object contexts.

So my question is - is using UnitOfWork directly from the entity acceptable at all. I've implemented the Data Access Layer but I've used a lot of information from different articles and I can't say that I fully understand the reason behind everything. However it seems to me that using it this way not only causes the code to crash but also is against the whole idea for using Repositories and UnitOfWork to manage your data. On the other hand the guy that has added this code has more than 10 years of programming experience so I can't just tell him he must change his code. So is it in fact OK to use unitOfWork from inside the entity itself as in the example above and if it's actually is, then how can I resolve the error with the different object contexts being used?

Answer 1

In my opinion (which isn't necessarily valid, I have little ASP experience), in this case, the entity definitely shouldn't be using the UnitOfWork, or even be aware of it.

If you already have configured Menu entities which entity framework is managing, and you have several existing repositories set up to further abstract & manage your database interactions, it doesn't make sense to pollute your model.

I'm assuming the error is appearing because the new UnitOfWork the other developer has defined in the model is a new context, and the Menu entity has already been obtained from another context (your MenuRepository), which you've already figured out.

It seems like the other developer is attempting to load the child MenuItems if there aren't any present in the Menu entity, when setting the MenuID. It's not entirely clear to me what's going on there, it seems as if they're expecting the item to have its ID changed, and then wanting to obtain the MenuItems belonging to Menu with the new ID (I'm not sure why you'd want to change the ID).

The MenuItem loading should be possible at the time the entity is retrieved, or afterward depending on how the entities are mapped/configured, using Lazy, Eager, or Explicit loading.

It's certainly worth talking to the other developer and finding out what they're trying to achieve, don't be worried about the amount of experience someone has (or says they have), many of these concepts may be new to them as well, or they may have spent all that time reinforcing bad habits, good programmers should always be willing to have a discussion about various approaches.

Some extra info on loading: http://msdn.microsoft.com/en-us/data/jj574232.aspx