views:

170

answers:

5

I have been reading though the code of the NerdDinner app and specifically the Repository Pattern... I have one simple question though, regarding this block

    public DinnersController()
        : this(new DinnerRepository()) {
    }

    public DinnersController(IDinnerRepository repository) {
        dinnerRepository = repository;
    }

What if each Dinner also had, say, a Category... my question is

Would you also initialize the category Repository in the constructor of the class??

Im sure it would work but Im not sure if the correct way would be to initialize the repository inside the method that is going to use that repository or just in the constructor of the class??

I would appreciate some insight on this issue

Thanks.

A: 

It depends on whether you will be testing your Controllers (, which you should be doing). Passing the repositories in by the constructor, and having them automatically injected by your IOC container, is combining convenience with straightforward testing. I would suggest putting all needed repositories in the constructor.

If you seem to have a lot of different repositories in your constructors, it might be a sign that your controller is trying to do too many unrelated things. Might; sometimes using multiple repositories is legitimate.

Edit in response to comment:

A lot of repositories in one controller constructor might be considered a bad code smell, but a bad smell is not something wrong; it is something to look at because there might be something wrong. If you determine that having these activities handled in the same controller makes for the highest overall simplicity in your solution, then do that, with as many repositories as you need in the constructor.

I can use myself as an example as to why many repositories in a controller is a bad smell. I tend to get too cute, trying to do too many things on a page or controller. I always get suspicious when I see myself putting a lot of repositories in the constructor, because I sometimes do try to cram too much into a controller. That doesn't mean it's necessarily bad. Or, maybe the code smell does indicate a deeper problem, but it not one that is too horrible, you can fix it right now, and maybe you won't ever fix it: not the end of the world.

Note: It can help minimize repositories when you have one repository per Aggregate root, rather than per Entity class.

Patrick Karcher
I dont understand why thats the case though... most of my controllers are simple CRUD.. its just that there are many linq relations.. some are many to many.. and for each model I have created a Repository (which I thought was good practice)
NachoF
+1  A: 

I would have my dinner categories in my dinner repository personally. But if they had to be seperate the id put them both in the ctor.

Paul Creasey
But wouldnt that prevent you from having a categories controller in which you can manage categories???... its the reason why I would think the Categories Repository is needed.
NachoF
+1  A: 

To keep your code decoupled and your controllers easily testable you need to stick with dependency injection so either:

public DinnersController() 
    : this(new DinnerRepository(), new CategoryRepository()) { 
} 

or the less elegant

public DinnersController() 
    : this(new DinnerRepository(new CategoryRepository())) { 
} 
sipwiz
thanks... it's what Im doing... just wasnt sure it was correct
NachoF
A: 

You'd want to pass it in to the constructor. That said, I probably wouldn't create any concrete class like it's being done there.

I'm not familiar with the NerdDinner app, but I think the preferred approach is to define an IDinnerRepository (and ICategoryRepository). If you code against interfaces and wanted to switch to say, an xml file, MySQL database or a web service you would not need to change your controller code.

Pushing this out just a little further, you can look at IoC containers like ninject. The gist of it is is that you map your IDinnerRepository to a concrete implementation application wide. Then whenever a controller is created, the concrete repository (or any other dependency you might need) is provided for you even though you're coding against an interface.

Andy Gaskell
"I'm not familiar with the NerdDinner app, but I think the preferred approach is to define an IDinnerRepository (and ICategoryRepository). If you code against interfaces and wanted to switch to say, an xml file, MySQL database or a web service you would not need to change your controller code."I thought that's what they are doing..
NachoF
Well, it's being done kind of halfway. The parameterless constructor is taking a dependency on a concrete class.
Andy Gaskell
+1  A: 

What you're looking at here is actually not so much to do with the repository pattern, per se, and more to do with "dependency injection," where the outside things on which this class depends are "injected" from without, rather rather than instantiated within (by calling new Repository(), for example).

This specific example shows "constructor injection," where the dependencies are injected when the object is created. This is handy because you can always know that the object is in a particular state (that it has a repository implementation). You could just as easily use property injection, where you provide a public setter for assigning the repository or other dependency. This forfeits the stated advantage of constructor injection, and is somewhat less clear when examining the code, but an inversion-of-control container can handle the work of instantiating objects and injecting dependencies in the constructor and/or properties.

This fosters proper encapsulation and improves testability substantially.

The fact that you aren't instantiating collaborators within the class is what improves testability (you can isolate the behaviour of a class by injecting stub or mock instances when testing).

The key word here when it comes to the repository pattern is encapsulation. The repository pattern takes all that data access stuff and hides it from the classes consuming the repository. Even though an ORM might be hiding all the actual CRUD work, you're still bound to the ORM implementation. The repository can act as a facade or adapter -- offering an abstract interface for accessing objects.

So, when you take these concepts together, you have a controller class that does not handle data access itself and does not instantiate a repository to handle it. Rather the controller accepts an injected repository, and knows only the interface. What is the benefit? That you can change your data access entirely and never ever touch the controller.

Getting further to your question, the repository is a dependency, and it is being provided in the constructor for the reasons outlined above. If you have a further dependency on a CategoryRepository, then yes, by all means inject that in the constructor as well.

Alternatively, you can provide factory classes as dependencies -- again classes that implement some factory interface, but instead of the dependency itself, this is a class that knows how to create the dependency. Maybe you want a different IDinnerRepository for different situations. The factory could accept a parameter and return an implementation according to some logic, and since it will always be an IDinnerRepository, the controller needs be none the wiser about what that repository is actually doing.

Jay
Superb summary. I'm sure NachoF's head is spinning right now. :)
bzlm