+972 52-548-6969
Thinking about repositories
For a long time, I have used the idea of IRepository<T> as my main data access method.
It is a very good approach, and I have used a static gateway to be able to resolve that effectively. The code to use that read like this:
Repository<Account>.Save(account);
But, I have an issue with that, because this is an opque dependency. I don't have a way to look at the declaration and just get what this needs. It makes testing a lot harder than it should be, because you now need to do indirect dependency injection.
There is the additional problem of the actual IRepository<T> interface:
That is an interface with 40 methods. And most of those methods are dealing directly with the data access code.
When I first to move away from this approach, I started to inject the IRepository<Account> into my services, which worked well. The code looked like this:
public class AccountService { public AccountService(IRepository<Account> accountsRepository) { //... } }
The problem with that is readability, take a look at the usage:
accountsRepository.FindAll(
Where.Account.LastActivity < DateTime.Now.AddDays(-60)
);Can you tell me what the meaning of this is? It express what it is doing really well, I think, but not what the meaning of it.
So, I started with the idea that I have domain repositories and infrastructure repositories. So I introduced this interface:
And then I can call:
accountsRepository.FindAllInvactiveAccounts()
I didn't really like that, because infrastracture repository doesn't really sit well with me. There is another issue, which is the implementation of IAccountsRepository implementation:
I have the IAccountRepository depending on IRepository<Account>, but that is... a smell. Then I thought about this approach:
But that doesn't sit very well with me either.
Eventually, I got to this design:
accountsRepository.FindAll(
new InactiveAccountQuery()
);Where the accountsRepository is a IRepository<Account> implementation. A better syntax for that is probably this:
accountsRepository.FindAll( Accounts.Inactive );
That looks much better, and where I am going to stop for now
Comments
I really like the simplicity and the elegance of this solution. Can u please expand about your thought (and actions) regarding the data access methods which are exposed through the IRepository? I find it even more disturbing than the readability issue.
Thanks
Time to move to LINQ, my friend . . .
Looks good Oren. I'm getting ready to start pushing a better repository model at work, so keep these kinds of posts coming! :)
I just thought I'd quickly post up that to MSFT, your "IAccountRepository depending on IRepository<Account>, but that is... a smell" is actually a recommendation. I can't recall the link off hand, but there is a framework design guideline that while closed generics can be used all over the shop within a part of a framework, they recommend deriving a non-generics type from that closed generic type if that type is to be exposed externally. Perhaps not exactly the same scenario as this, but not all that far off if you think about it, all coming down to who and how far you would consider "external".
PS - they even recommended this for your code that externally exposes things like List<MyClass>, so the recommendation covered both your own generics types as well as MSFTs.
I am not sure that I follow you here, can you target the question a bit?
Jeremy, for me this is busywork without much value.
Agreed, I find it to be busywork as well. I just thought it interesting that this busywork would actually make for a recommendation in some scenarios. I can imagine that if the closed generic type was rather complex in nature that it might even prove useful to create a dedicated derivation even if only for use internally, but I expect that the general guidance was intended primarily for use when creating APIs that are to be used by an audience for which generics might be too complex.
Not that agree with making excuses for such audience members. I'd rather they become familiar with generics :) but can understand that sometimes an API might be improved by tucking them away a bit.
Interesting, but how is "AccountInactiveQuery" class implemented
and how do you decouple NHibernate specifics from the IRepository<T>?
Can you give an example of what the full chain of execution is, starting from the client that uses this service?
And my last question:
Let's assume that your AccountService is a closed WebService and I have a web application that wants to use your service. Let's also assume that you hadn't provided any method in the service for returning the InactiveAccounts collection. How can I create on the client the same InactiveAccounts query specification and send it on the service for execution? (It would be a waste to call accountService.GetAllAccounts() and filter in-memory on the client)
I can think of a QueryDTO that can be sent on the service for the specific entity, but many times, you have Search forms with multiple ways to define complex criteria, and your service may not have all the knowledge of specific use case you want to support (because I don't have source code access to it), so the service should provide a generic way for the client to specify query criteria and execute them returning the results.
Anyway, maybe I'm missing something here, excuse the long questions. I'll be happy for any information regarding the above.
Probably by calling NH directly
I don't need to, I have an expressive way to declare intent, that is what I am after.
That is the wrong approach. Web Services are not supposed to be CRUD. Web Services should carry some meaning.
You don't have an UpdateCustomer() method, you have UpdateCustomerAddressMessage.
I use a similar approach, but with a static "QueryFactory" that constructs all of the queries for one entity:
Might be like:
AccountQueryFactory.Inactive()
AccountQueryFactory.InactiveForDays(int numberOfDays)
AccountQueryFactory.ByEmailAddress(string email)
etc...
I like it because it provides one place for developers to find and reuse existing queries (with intellisense) rather than creating a new query for each new bit of functionality... and one place to test/fix when making database changes.
I could see where this could get unwieldy for large, complex applications, and having the queries be unique to each client application would be more beneficial.
Also, in some cases I might use code generation to spit out query factory methods based on the schema, for example: queries on each foreign key and each index on the underlying table.
(Side note: One really nice thing about the code-generated fluent queries is that they cause a compile-breaking change when the schema is out-of-sync with your app code... very cool)
@Ayende - I've been using the approach you suggested for quite a while and have found it useful. The big cloud for me is over inheritance: can, and should you use the 'base' repository when dealing with subclasses?
It is quite easy to do IVehicleRepository.Save(aCar) when Car is actually a subclass of Vehicle. Of course sometimes it is even desirable for a repository to work with an interface, rather than a concrete type...
The new approach I'm tending towards is more like:
What do you think? Of course if you want separate implementations for different entity types you have to write some delegating smarts into your implementation, but otherwise, it doesn't seem too bad.
The .NET world is due for a whole new argument of course about whether or not LINQ queries leak data access concerns, but considering that you can query a collection on an existing business object using exactly the same syntax, types and (roughly) semantics, I'm happy enough exposing it.
Nicholas,
My repositories are almost always bound to a database, and I usually have a single implementation that handles everything there.
Hi Ayende,
My repositories are bound to databases too, I presume you were referring to my comment about querying collections (sorry - might have seemed a little OT.)
I've been asked by a few people about whether exposing an IQueryable<T> to other services outside of a "data access layer" is the equivalent of spreading SQL around a system built on ADO.NET.
My current thinking is that it is not, because of the opportunities for mocking e.g. in the case of using an in-memory collection for the purposes of unit testing, and because the 'schema' is that of the domain model, which is already exposed.
(BTW, don't you think that it is pretty neat that using LINQ we can potentially test queries without a database at all...? ...but I digress :))
I don't need linq for testing queries with DB.
Check my post about specifications, that cover query object building, I think.
Can you explain why you don't like
accountsRepository.FindAllInvactiveAccounts()
We use that type of repository pattern and have found it to work out quite well for us. Of course that does not mean that there is something wrong with it and we are not seeing it. :D
Joe,
the problem is not with this code:
accountsRepository.FindAllInvactiveAccounts()
The problem is with the associated implementation pattern, how you inject IRepository<T> into the mix, for instance.
I am in Joe's camp as well, although not sure of the implementation similarities. I have an IAccountsRepository inherit from IRepository<Account>, and an AccountsRepository inherit from Repository<Account>. I'm not sure the objection of "injecting IRepsitory<T> into the mix".
In the diagram above where you show this, you say it doesn't "sit well". I'm curious as to what it is that doesn't sit well with you, and what you're trying to improve.
Mark, the problem is that it means that I need to do extra work. I don't like that. The architecture should not encourage me to to extra to get the right design, it should be built in a way that ensure that the easy way is the right way.
Ok, I've woken up now, and think I see what you're trying to say with using the query object passed right to the IRepsitory<T>, which originally I didn't see.
While a full-blown criteria implementation as you have shown in past blog posts might be pure overkill for my project, perhaps something based around DetachedCriteria could work for me. I'm working on a Greenfield project and might try something like this out.
Hi,
I'm currently playing around with a new way of DAO (you call it Repository), which is similar to your setup. I have an IDAO<T> interface and a generic implementation GenericDAO<T> : AbstractDAO<T>, IDAO<T> which can handle simple stuff like Save, Load, FindAll. These features are inherited from an abstract class AbstractDAO.
For more domain specific approaches I create special DAO like TasksDAO : AbstractDAO<Task>, IDAO<Task>, which has a method like GetTasksForUser(string userName). Within this method I construct my NHibernate criterias and pass them to some protected method of the AbstractDAO. This way only the DAO objects have a reference to NHibernate, my application (Web, Console, ...) does not need to know.
To get the various DAOs I use castle windsor to look them up. So usually I will just request an IDAO<Task> myTaskDao = container.Resolve<IDAO<Task>>();. In case I need some "special" features I can get TaskDAO myTaskDao = (TaskDAO)container.Resolve<IDAO<Task>>();.
@Henning.
That's pretty much the way I've been doing it too (in fact, I have been calling them DAO's as well).
What Ayende is saying is that the necessity to create a new interface for each entity with specialized queries seems like additional overhead. I've been trying out using a more "query object" approach, where each query is it's own class, and having a method or two on your dao used specifically for these queries. So, here is my one dao interface, which has one dao implementation...
public interface IDao<T>
{
___// All the typical Save, Load, Update, Delete methods..
___T Find(Query<T>);
___ISet<T> FindAll(SetQuery<T>);
}
You could probably just always return a set or list for the query, and ignore having two queries (Query and SetQuery), but for now that's what I'm going with. The Query and SetQuery classes are simply abstract classes with an abstract method RunQuery(ISession);. They could just be interfaces but I also have some "helper" methods on them.
Whenever I want to write a query, I would do...
public class FindAllAccountsOwnedBy : SetQuery<Account>
{
__private readonly int ownerId;
__public class FindAllAccountsOwnedBy(int ownerId)
__{
____this.ownerId = ownerId
__}
__public ISet<T> RunQuery(session)
__{
____ISet<T> results;
____// Run query using whatever method I enjoy, and return results
__}
}
Running the query...
ISet<Account> results = ISet<Dao>.FindAll(new FindAllAccountsOwnedBy(id));
Some things I've noticed so far...
1.) If you're ok with just this, you only need to name the query once in the class. You don't need to do it again in the interface. You may need to a second time if you have the additional static builder class that Ayende suggested above to return the new query object, but that's not too difficult. The thing that makes that duplicated effort different than the interface naming duplicated effort is...
2.) Whenever I created a new dao, I would need to add that dao in windsor. This can get annoying, especially if I ever needed to shift around namespaces of these daos later on, and the service configurations were in XML. Using query objects, only the abstrct dao must be used in windsor. The query objects are created on their own and thus do not require Windsor whenever you create a new one. Thus, one line of windsor xml for EVERY query in your system.
3.) While not currently on my list, I'm looking to eventually find some way of using daos in a manner that don't require NHibernate, allowing for faster early development. For example, using an in-memory store of just plain list objects, rather than having to come up with nhibernate mapping files, etc. when I'm still showing demos. If I were to go that way, by making a specialized dao for each object I'd have to duplicate the work of each query in the InMemory dao and the NHibernate dao. Instead, by using a specification object, you can create the query, and have both the InMemory dao and NHibernate dao only need to create one method each for all queries that handle these specification queries.
4.) It's much easier to working with maintaining queries, as you will only have one query in your window at a time, rather than having them stacked one on top of another as in a larger query class. For me, it's much easier to find the code I'm looking for and concentrate on it.
That being said, it might not be worth it to try to duplicate this into a larger project with tons of daos already created.
@Henning.
That's pretty much the way I've been doing it too (in fact, I have been calling them DAO's as well).
What Ayende is saying is that the necessity to create a new interface for each entity with specialized queries seems like additional overhead. I've been trying out using a more "query object" approach, where each query is it's own class, and having a method or two on your dao used specifically for these queries. So, here is my one dao interface, which has one dao implementation...
public interface IDao<T>
{
___// All the typical Save, Load, Update, Delete methods..
___T Find(Query<T>);
___ISet<T> FindAll(SetQuery<T>);
}
You could probably just always return a set or list for the query, and ignore having two queries (Query and SetQuery), but for now that's what I'm going with. The Query and SetQuery classes are simply abstract classes with an abstract method RunQuery(ISession);. They could just be interfaces but I also have some "helper" methods on them.
Whenever I want to write a query, I would do...
public class FindAllAccountsOwnedBy : SetQuery<Account>
{
__private readonly int ownerId;
__public class FindAllAccountsOwnedBy(int ownerId)
__{
____this.ownerId = ownerId
__}
__public ISet<T> RunQuery(session)
__{
____ISet<T> results;
____// Run query using whatever method I enjoy, and return results
__}
}
Running the query...
ISet<Account> results = ISet<Dao>.FindAll(new FindAllAccountsOwnedBy(id));
Some things I've noticed so far...
1.) If you're ok with just this, you only need to name the query once in the class. You don't need to do it again in the interface. You may need to a second time if you have the additional static builder class that Ayende suggested above to return the new query object, but that's not too difficult. The thing that makes that duplicated effort different than the interface naming duplicated effort is...
2.) Whenever I created a new dao, I would need to add that dao in windsor. This can get annoying, especially if I ever needed to shift around namespaces of these daos later on, and the service configurations were in XML. Using query objects, only the abstrct dao must be used in windsor. The query objects are created on their own and thus do not require Windsor whenever you create a new one. Thus, one line of windsor xml for EVERY query in your system.
3.) While not currently on my list, I'm looking to eventually find some way of using daos in a manner that don't require NHibernate, allowing for faster early development. For example, using an in-memory store of just plain list objects, rather than having to come up with nhibernate mapping files, etc. when I'm still showing demos. If I were to go that way, by making a specialized dao for each object I'd have to duplicate the work of each query in the InMemory dao and the NHibernate dao. Instead, by using a specification object, you can create the query, and have both the InMemory dao and NHibernate dao only need to create one method each for all queries that handle these specification queries.
4.) It's much easier to working with maintaining queries, as you will only have one query in your window at a time, rather than having them stacked one on top of another as in a larger query class. For me, it's much easier to find the code I'm looking for and concentrate on it.
That being said, it might not be worth it to try to duplicate this into a larger project with tons of daos already created.
Sorry about the double post :P
Mark,
this is an interesting approach, using only one (!) gerneric dao and supply the queries (you could probably supply the query-object of your DAO by a delegate?!?).
But still you will have to create those query classes; to me I currently don't see so much benefit from creating a seperate class for the queries instead of using a specialized DAO. I see the point about the windsor-config though ... but nethertheless you will have to create some custom code - and just like the DAO the query-object has to have a reference to NHibernate (for example).
Actually I would like to have this a little different. I would like to have some kind of generic (maybe static?) DAO that deals with the NHibernate stuff. Then I would like to have methods on my business-objects, which would express some kind of queries, without having to reference NHibernate.
For example:
I have an order business-object and I want to get all outstanding orders for a customer. So now I would need to create a special DAO for orders with a method "GetOutstandingOrders(Customer c)" (or I create a query which would accomplish the same and use the DAO you proposed).
What I would rather prefer: I have a method "GetOutstandingOrders(Customer c)" on my order object. The logic to identify the outstanding orders for a customer would be business-logic and not data-access-logic, so that's why I would like to see it within the business-layer along with my business-objects instead of having to code this in the data-layer.
But this would require to have some way to express queries independend of NHIbernate (or whatever framework).
I have a method "GetOutstandingOrders(Customer c)" on my order object - that sounds like a violent violation of Single Responsibility Principle.
Comment preview