4

I have the first large solution that I am working on using MVC3. I am using ViewModels, AutoMapper, and DI.

To create my ViewModels for some of the more complex edit/creates I am injecting 10 or so
repositories. For all but one of the repositories they are only there to get the data to populate a select list on the ViewModel as I am simply getting associated FK entities etc.

I've seen it mentioned that injecting large numbers of repositiories is bad practice and I should refactor. How many is to many? is this to many? How should I refactor? Should I create a dedicated service that returns select lists etc?

Just to to give an example here is the the constructor for my RequirementsAndOffer Controller

       public RequirementsAndOfferController(
IdefaultnoteRepository defaultnoteRepository, 
IcontractformsRepository contractformsRepository, 
IperiodRepository periodRepository, 
IworkscopeRepository workscopeRepository, 
IcontactRepository contactRepository, 
IlocationRepository locationRepository, 
IrequirementRepository requirementRepository, 
IContractorRepository contractRepository, 
IcompanyRepository companyRepository, 
IcontractRepository contractRepository, 
IrequirementcontracttypeRepository requirementcontracttypeRepository, 
IoffercontractRepository offercontractRepository)

All of the above populate the selects apart from the requirementRepository and offercontractRepository which I use to get the requirements and offers.

Update General thoughts and updates. I was encouraged to consider this issue by Mark Seemann blog article on over injection. I was interested in specifically the repositories and why I was having to inject this number. I think having considered my design I am clearly not using one repository for each aggregate root (as per DDD).

I have for example cars, and cars have hire contracts, and hire contracts have hire periods.

I was creating a repository for cars, hire contracts, and hire periods. So that was creating 3 repositories when I think there should only be one. hire contracts and periods can't exist without cars. Therefore I have reduced some repositories that way.

I am still left with some complex forms (the customer is demanding these large forms) that are requiring a number of repositories in the controller. This maybe is because I haven't refactored enough. As far as I can see though I am going to need separate repositories to get the select lists.

I'm considering options for creating some sort of service that will provide all the select lists I need. Is that good practice/bad practice? Should my services only be orientated around aggregate roots? If so having one service providing selects would be wrong. However the selects do seem to be the same type of thing and grouping them together is attractive in some ways.

Would seem my question is similar to how-to-deal-with-constructor-over-injection-in-net

I guess I am now more looking for specific advice on whether a Select List service is good or bad.

Any advice appreciated.

GraemeMiller
  • 11,973
  • 8
  • 57
  • 111
  • I think the issue here is related to your domain design. In DDD - Domain Driven Design - you don't necessarily have a repository for each entity in your domain. There is the concept of Aggregates where several domain types are grouped together into an aggregate with one type relegated as a root. You would create an repository for each aggregate then. I would recommend reading this mini-book for an excellent intro to DDD http://www.infoq.com/minibooks/domain-driven-design-quickly. – Waleed Al-Balooshi Sep 15 '11 at 20:23
  • Thanks getting the book. I did wonder if that was an issue as I had seen much talk of aggregate roots. However all of the things that are repositories the end user can directly add/create/edit etc. Although many of them are largely used to populate select lists on various views. E.g I have a locations as an entity and various entities have locations associated with them so it seemed logical that it needed a repository of its own so people could add locations etc? – GraemeMiller Sep 15 '11 at 20:28
  • As I don't have experience in the domain that you are designing for, I ufortunately can't provide direct comments on the design. But as an example, lets look at contracts and periods. To me a period doesn't exist on its own - and I would consider it a value object -, but is part of a contract. So, contract could be an aggregate root with period being a child of that aggregate. I would then work with the period through contract. You would have a contract repository that takes care of both the contract and period. Of course, based on your domain this might not be the case. – Waleed Al-Balooshi Sep 15 '11 at 21:01
  • Also, if you want an even quicker guide on DDD and to get an intro to the various components of DDD take a look at this article from MSDN magazine http://msdn.microsoft.com/en-us/magazine/dd419654.aspx#id0090074 – Waleed Al-Balooshi Sep 15 '11 at 21:03
  • Yeah, I will re-read the DDD stuff and see if I should be reducing the number of repositories. – GraemeMiller Sep 15 '11 at 22:13
  • possible duplicate of [Dependency Injection Constructor Madness](http://stackoverflow.com/questions/2420193/dependency-injection-constructor-madness) – Mark Seemann Sep 16 '11 at 07:37
  • Well I suppose you could say that but I was more concerned about the specific fact they are repositories and was that a problem in my design etc. It was actually that blog that made me think what was doing was bad. I think my problem is I have to many repositories and need to refactor them into less (aggregate root etc). Although I wonder if I need to add a service as well – GraemeMiller Sep 16 '11 at 15:52
  • Having thought about this and reading Marks blog (which put me onto the idea that I was being bad) and his comment "One of the wonderful benefits of Constructor Injection is that it makes violations of the Single Responsibility Principle glaringly obvious." I have decided to follow Waleeds advice and reduce my number of repostiories by refacotring to one repository per aggregate root. I'm still going to have to consume some repositories on pages to get SelectList. Would creating some sort of SelectList providing service be good or bad practice? – GraemeMiller Sep 16 '11 at 22:39

1 Answers1

3

You have the right idea starting with a repository pattern. Depending on how you use your repositories, I completely understand how you might end up with a lot (maybe even 1 per database table.)

You'll have to be the judge of your own specifications and business requirements, but perhaps you can consider a business layer or service layer.

Business Layer

This layer might be composed of business objects that encapsulate one or more intents of your view models (and inherently views.) You didn't describe any of your domain, but maybe a business object for Users could include some CRUD methods. Then, your view models would rely on these business objects instead of directly calling the repository methods. You may have already guessed the refactoring would move calls to repository methods into the business objects

Service Layer

A service layer might even use some of the business objects described above, but perhaps you can design some type of messaging protocol/system/standard to communicate between your web app and maybe a WCF service running on the server to control state of some kind.

This isn't the most descriptive of examples, but I hope it helps with a very high level view of refactoring options.

David Fox
  • 10,603
  • 9
  • 50
  • 80
  • Maybe my issue is how I use the repositories? Basically I have one for each entity that I would use. Each of these are need so that people can do the CRUD operations for each entity. E.g. edit the possible contract types etc. I have implemented a GetSelectList for each of the repositories. So I thought about basically having a SelectList service that consumed all appropriate repositories and then only returned select lists. Then I could inject the selectlist service rather than lots of repositories. However that only removed the repository injection away from the Controlller. – GraemeMiller Sep 15 '11 at 12:53
  • @GraemeMiller In non-trivial applications, it's not the controller's responsibility to do CRUD anyway. So, even this refactoring could prove beneficial, more semantically correct and separate yet another concern. – David Fox Sep 15 '11 at 17:31
  • Thanks. Sorry I meant that the controller just returns the view model to allow people to do CRUD. All the actual operations that do it are in the appropriate repositories. – GraemeMiller Sep 15 '11 at 19:45
  • Followed various advice, rewrote repositories to be based on aggregate roots. Then took your/Marks advice and implemented aggregate services based on the roots. – GraemeMiller Sep 17 '11 at 15:41