How to handle different responsibilities while usi

2019-07-31 15:04发布

问题:

I have been looking around for an answer to my question but couldn't find one, mostly because I don't really know how to phrase it!

I am playing around with EF code first and trying to implement some sort of repository pattern whilst using dependency injection (thanks to Unity). I am also trying to keep to SOLID (well, the SRP part at least) in that I have IStaffRepository and IDepartmentRepository interfaces which in turn implement IRepository<TEntity> which provides basic CRUD methods.

My question is, how can I keep to the SRP when I need to update a staff members' department?

I have read that it is bad practice to use a single DbContext instance for all repositories due to possible memory leaks and so I cannot simply call the IDepartmentRepository to get the new department as this would use a separate instance of my DbContext.

The obvious solution to me is to include something like...

Department GetDepartment(int id);

Within IStaffRepository - but would this not break the SRP?

The code I have in my Controller is...

private IStaffRepository repository;
private IDepartmentRepository departmentRepository;

public StaffController(IStaffRepository repository, IDepartmentRepository departmentRepository)
{
   this.repository = repository;
   this.departmentRepository = departmentRepository;
}

public ActionResult Edit(int id)
{
   return View(new StaffViewModel(repository.GetItem(id)));
}

[HttpPost]
public ActionResult Edit(int id, StaffViewModel model)
{
   if (!ModelState.IsValid)
      return View(model);

   var item = repository.GetItem(id);

   // throws error due to different context
   item.Department = departmentRepository.GetItem(int.Parse(model.SelectedDepartment));

   UpdateModel(item, "Item");

   repository.Save();

   return RedirectToAction("Member", new {id});
}

And the StaffMember model contains...

public class StaffMember
{
   public virtual Department Department { get; set; }
}

And the StaffViewModel looks like this...

public class StaffViewModel : ViewModelBase<StaffMember>
{
   public SelectList DepartmentList {get;set;}
   public string SelectedDepartment {get;set;}

   public StaffViewModel()
   {
      var departmentRepository = new DepartmentRepository();
      DepartmentList = new SelectList(departmentRepository.GetCollection(), "ID", "Title", SelectedDepartment);   
   }

   public StaffViewModel(StaffMember item) : this()
   {
      Item = item;
      SelectedDepartment = Item.Department.ID.ToString();
   }

   public StaffViewModel(List<StaffMember> collection) : this()
   {
      Collection = collection;
   }
}  

In the database there is a int Department_ID field which hooks up to the Department table.

I have a drop down in my view which looks like...

@Html.DropDownListFor(m => m.SelectedDepartment, Model.DepartmentList, "--Please Select--")

Apologies for the length of this question!

回答1:

If all of your models are based on entities in the same database, there is not reason not to use a single context instance to manage them within the same controller. I would go with the simple solution and use a single context instance.



回答2:

If you're using the Repository pattern then you have at least a class which implements one of more repository interfaces. Since your code is coupled to an interface and not to an implementation you don't have to worry. However, you should treat the repository implementation as a singleton i.e every method should be independent and the class should not store state (you can store the conenction string since is basically a constant). So a method look like this

public void Save(Entity entity)
{
   using(var db=new MyDbContext())
   {
      //do stuff
   }
}

This way you're always disposing the data context and thus there should be no memory leaks As for SRP, the role of an repository is to get/save objects. If it does that it's good.



回答3:

The problem potentially with a single instance is not releasing the context when it is done. If you use dependency injection, this will handle the disposing of this object when it is done. I do not recommend the design you have above either - you are giving your ViewModel data access behavior, which generally they aren't purposed for. A ViewModel has properties for the model and MAYBE some functionality specific to prepare that data. Inject a service layer into your controller whose responsibility it is to populate that viewmodel OR return a model. You can then just use automapper to map that model to your ViewModel. There's a neat attribute available as well to do this mapping.

Check out: http://lostechies.com/jimmybogard/2009/06/30/how-we-do-mvc-view-models/