views:

85

answers:

2

Initial Warning: Long post and I might have got the pattern totally wrong anyway:

Given the following class which is the start of the Customer Aggregate:

public class Customer : KeyedObject
{

   public Customer(int customerId)
   {
      _customerRepository.Load(this);
   }

   private ICustomerRepository _customerRepository = IoC.Resolve(..);  
   private ICustomerTypeRepository = _customerTypeRepository = IoC.Resolve(..);

   public virtual string CustomerName {get;set;}
   public virtual int CustomerTypeId [get;set;}

   public virtual string CustomerType
   {
      get
      {
         return _customerTypeRepository.Get(CustomerTypeId);
      }
   }

}

And CustomerType is represented by a value object:

public class CustomerType : ValueObject
{
   public virtual int CustomerTypeId {get;set;}
   public virtual string Description {get;set;}
}

This is all well and good for when I have a customer object with a CustomerTypeId. However, when I want to populate a DropDownList within my MVC View, I'm struggling with the concept of how to correctly get the CustomerType value list from the ICustomerTypeRepostory.

The ICustomerTypeRepository is very simple:

public interface ICustomerTypeRepository
{
   public CustomerType Get(int customerTypeId);
   public IEnumerable<CustomerType> GetList();
}

Basically, what I want is to be able to call ICustomerTypeRepository correctly from my Controller, however I thought it would be best to separate the DAL (repository) layer from the controller. Now, am I just overcomplicating things?

This is how my controller currently stands:

public class CustomerController : ControllerBase
{ 

    private ICustomerTypeRepository _customerTypeRepository = IoC.Resolve(..);

    public ActionResult Index()
    {
       Customer customer = new Customer(customerId); 
       IEnumerable<CustomerType> customerTypeList = 
          _customerTypeRepository.GetList();

       CustomerFormModel model = new CustomerFormModel(customer);
       model.AddCustomerTypes(customerTypeList );
    }
}

This seems wrong to me as I've got repositories in the Controller and in Customer. It just appears logical to me that there should be a segragated access layer for CustomerType. I.e. CustomerType.GetList():

public class CustomerType : ValueObject
{
   // ... Previous Code

   private static ICustomerTypeRepository _customerTypeRepository = IoC.Resolve(..);

   public static IEnumerable<CustomerType> GetList()
   {
      _customerTypeRepository.GetList();
   }
}

So, which is the way I should be exposing the CustomerType objects through the ICustomerTypeRepository to the CustomerController?

A: 

How about changing your customer domain model to include a property for CustomerTypes? This would also stop the need to hit the repository each time CustomerType is called.

public class Customer : KeyedObject
{

   public Customer(int customerId)
   {
      _customerRepository.Load(this);

      ICustomerTypeRepository _customerTypeRepository = IoC.Resolve(..);
      _customerTypes = _customerTypeRepository.GetList();
   }

   private ICustomerRepository _customerRepository = IoC.Resolve(..);  

   public virtual string CustomerName {get;set;}
   public virtual int CustomerTypeId {get;set;}

   public virtual string CustomerType
   {
      get
      {
         return _customerTypes.Find(CustomerTypeId);
      }
   }

   private IEnumerable<CustomerType> _customerTypes;
   public virtual IEnumerable<CustomerType> CustomerTypes
   {
      get
      {
          return _customerTypes
      }
   }
}
Tim Murphy
You see, that's the thing. It seems wrong to have to load a Customer in order to access a list of CustomerTypes, i.e. to just populate a DropDown on the form.
GenericTypeTea
In your question the CustomerController.Index loads the customer types into CustomerFormModel so I assumed that you needed the customer and the list of customer types in the same model. Is that not the case? Maybe I'm thick or the question needs a bit of rewording.
Tim Murphy
I suppose it'd make sense in one way. But what if I just had an admin form where I wanted to add edit CustomerTypes? There'd be no concept of a Customer at that point, so I wouldn't want to load a Customer object.
GenericTypeTea
On the admin form you use CustomerTypes repository. Obviously I don't understand your problem.
Tim Murphy
+2  A: 

I think there are a few things to consider here.

First of all, if you are really interested in modeling your domain, you'll want to try at all costs to keep domain entities themselves free of cross-cutting concerns, like validation, IoC containers, and persistence, the Active Record pattern notwithstanding.

What this means is that Customer should probably not have any reference to a repository, even if you're using interfaces and a service locator. It should be designed to reflect the attributes of -- or what constitutes -- a "customer" from the perspective of your target client/user.

Domain-modeling aside, I'm a little concerned by the use of your IoC service locator in a variable initializer. You forfeit any opportunity to catch exceptions and exceptions thrown by constructors are notoriously hard to debug (these initializers run before any code in your first non-static constructor).

The use of the static gateway/service locator in lieu of injecting dependencies also renders the class virtually untestable (using automated unit testing methodologies, that is -- you can do integration and manual testing, but test failures are unlikely to point you to the broken bits readily -- as opposed to a unit test, where you know exactly what one thing you are testing and therefore what is broken).

Instead of having the Customer object populate itself with data by calling _customerRepository.Load(this) in the constructor, an application will more typically use the repository to get the entity, so it comes back from the repository wholly populated, including the CustomerType property. In this case, it appears that this might occur in the CustomerController.

You indicate that you would want to have a separate DAL from the layer in which the CustomerController resides, and you effectively have that -- this is where using a repository interface comes in. You inject some implementation of that interface (or in this case, get an implementation from the IoC implementation), but the actual implementation of that repository can exist in a separate layer (it can be another assembly even). This is a pattern known as Separated Interface.

Personally, I would refactor the CustomerController to look like this:

public class CustomerController : ControllerBase
{ 
     private ICustomerTypeRepository _customerTypeRepository;
     private ICustomerRepository _customerRepository;

     public CustomerController(ICustomerRepository customerRepository,
        ICustomerTypeRepository customerTypeRepository)
     {
        _customerRepository = customerRepository;
        _customerTypeRepository = customerTypeRepository;
     }

     public ActionResult Index()
     {
         Customer customer 
             = _customerRepository.GetCustomerWithId(customerId); 
             // from where does customerId come?

         IEnumerable<CustomerType> customerTypeList 
             = _customerTypeRepository.GetTypes();

        . . .

     }
}

…and I'd take any and all references to repositories out of Customer and any other domain entity class.

Jay