4

I have an ASP.NET MVC4 web application, with EntityFramework for data access. In the application I have two repositories as follow (this is only an example not my real code):

public class CustomerRepository
{
     ICustomerContext _db;

     public CustomerRepository(ICustomerContext db)
     {
          _db = db;
     }

     ...

     public void DeactivateCustomer(int customerId)
     {
         var customer = _db.Customers.Single(x => x.CustomerId == customerId);
         customer.IsActive = false;
         _db.SaveChanges();
     }
}

And

public class ItemRepository
{
     IItemContext _db;

     public ItemRepository(IItemContext db)
     {
         _db = db;
     }

     ...

     public void DeactivateItem(int itemId)
     {
         var item = _db.Items.Single(x => x.ItemId == itemId);
         item.IsActive = false;

         // The problem is here
         new CustomerRepository().DeactivateCustomer(item.CustomerId);

         _db.SaveChanges();
     }
}

The ItemRepository wants to use the logic of deactivating the Customer from the CustomerRepository without copying the code, or couple both repositories like the example above.

I also want to make both actions (deactivating the item, and deactivating the customer) execute in one transaction (maybe one call to SaveChanges())

Is there any design pattern that can achieve these requirements?

2
  • 2
    you should move the deactivateCustomer call and the transaction open/close up a level to the calling code
    – Ewan
    Oct 23 '15 at 13:11
  • What you're missing is a service class that consumes both of these to do work. In my opinion repositories shouldn't be talking to each other. Jan 21 '16 at 22:39
1

Probably best way to address this is to use a unit of work pattern, and use a business logic layer to handle these.

public class UnitOfWork : IDisposable
{
    private ICustomerContext customerContext;
    private IItemContext itemContext;

    private CustomerRepository customerRepository;
    private ItemRepository itemRepository;

    public CustomerRepository CustomerRepository
    {
        get
        {

            if (this.CustomerRepository == null)
            {
                this.CustomerRepository = new CustomerRepository (customerContext);
            }
            return CustomerRepository;
        }
    }

    public ItemRepository ItemRepository
    {
        get
        {

            if (this.ItemRepository == null)
            {
                this.ItemRepository = new ItemRepositoryitemContext);
            }
            return ItemRepository;
        }
    }

    public void Save()
    {
        customerContext.SaveChanges();
        itemContext.SaveChanges();
    }

    private bool disposed = false;

    protected virtual void Dispose(bool disposing)
    {
        if (!this.disposed)
        {
            if (disposing)
            {
                context.Dispose();
            }
        }
        this.disposed = true;
    }

    public void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }
}

Now in your business layer, you could simply.

public class ItemWorker 
{
     public void DeactivateItem(int itemId)
     {
         using (var unitOfWork = new UnitOfWork)
         {
             var item = unitOfWork.ItemRepository.GetItem(itemId); 


             unitOfWork.ItemRepository.DeactivateItem(itemId);
             unitOfWork.CustomerRepository.DeactivateCustomer(item.CustomerId);
             unitOfWork.Save();
         }
     } 
}

Reference

1

In my experience, trying to make a framework like EF work in a repository pattern is not the best if you really want to bundle transaction and separate functionality out into Repo's like that. I would have the MVC controller instantiate an EF data context, and be responsible for calling "SaveChanges()" after its done any active-record data manipulations. For DRY's sake, you can move all those manipulation functions to a common library that takes IItemContext as a method or constructor param. That lib might look something like:

public class IItemDataManipulator
        {
            private readonly IItemContext _db;

            public IItemDataManipulator(IItemContext db)
            {
                _db = db;
            }

            public void DeactivateCustomer(int custId)
            {
                var customer = _db.Customers.Single(x => x.CustomerId == customerId);
                customer.IsActive = false;
            }

            public void DeactivateItem(int itemId)
            {
                var item = _db.Items.Single(x => x.ItemId == itemId);
                item.IsActive = false;
                DeactivateCustomer(item.customerId);
            }
        }

And I wouldn't bother adding single-line EF queries to that library for basic object retreival. If you want to lock things down more, put this class in the same library as the EF datacontext and mark all the setters INTERNAL for stuff that you ONLY want to be able to change inside this common library.

0

The logic of deactivating a customer should exist on a Customer class. Only after pulling the customer from the database and modifying its value should you then save it back to the database. Furthermore, the logic of deactivating an item should go on the Item class. To make this happen, you need a more descriptive relationship between Item and Customer:

public class Customer
{
    public bool IsActive { get; set; }
    public long CustomerId { get; set; }

    public void Deactivate()
    {
        IsActive = false;
    }
}

public class Item
{
    public bool IsActive { get; set; }
    public long CustomerId { get; set; }
    public Customer Customer { get; set; }

    public void Deactivate()
    {
        IsActive = false;

        if (Customer != null)
        {
            Customer.Deactivate();
        }
    }
}

Now your repository classes don't need this business logic -- in fact repositories should not have any business logic in them for the exact same reason that you are posting this question. It's hard to reuse business logic. The real change happens in the code using the repository classes:

public class ItemsController : Controller
{
    public ActionResult Deactivate(long id)
    {
        using (var db = new DbContext())
        {
            var item = db.Items.Find(id);

            item.Deactivate();

            db.SaveChanges();
        }
    }
}

(using Entity Framework as an example)

  1. Create the database context object inside a using block so it gets properly disposed if an exception gets thrown
  2. Fetch the Item from the database context
  3. Call Deactivate() on the item
  4. The item deactivates itself, then calls Deactivate on the Customer.
  5. End the transaction by calling db.SaveChanges() to persist any domain object changes to the database

If you aren't using Entity Framework, you might need to explicitly save the Item and Customer objects separately in the controller, but the main design pattern still applies.

2
  • 1
    Thank you for your answer.. I just don't want to add business logic to these classes for some reasons.. 1) These classes are only POCO entities used by EF.. 2) They're mainly auto-generated by reverse engineering the database tables.. 3) Having my business logic scattered in so many entities makes it hard to maintain and review.. 4) Some logic is too complex to be placed in a single entity and it will not be intuitive to call it from an instance of that entity.
    – KeyBored
    Oct 23 '15 at 20:48
  • Anyway, I'm not restricted to the Repository Pattern, I can use any other pattern that can help me reuse my business logic easily and also keep that logic centralized as possible.
    – KeyBored
    Oct 23 '15 at 20:49
0

Do not do it directly with the DB context, that will lead to poor code reuse. Also, do not instantiate a repository directly inside another repository, trust me, it will bite you in the butt later.

Your repository should look more like this, with the concrete child of IRepoFactory defined in the calling application:

public class MyRepo
{
     IRepoFactory _factory;
     public MyRepo(IRepoFactory factory)
     {
          _factory = factory;
     }

     public void DoThing()
     {
          _factory.GetMyOtherRepo().DoOtherThing();
     }
}

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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