Ayende @ Rahien

Weex commented on Thinking about repositories

Fri, 07 Dec 2007 12:33:25 GMT

I have a method "GetOutstandingOrders(Customer c)" on my order object - that sounds like a violent violation of Single Responsibility Principle.

Henning Eiben commented on Thinking about repositories

Fri, 07 Dec 2007 09:30:55 GMT

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).

Mark Hildreth commented on Thinking about repositories

Thu, 06 Dec 2007 23:18:54 GMT

Sorry about the double post :P

Mark Hildreth commented on Thinking about repositories

Thu, 06 Dec 2007 23:17:30 GMT

@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 { ___// All the typical Save, Load, Update, Delete methods.. ___T Find(Query); ___ISet FindAll(SetQuery); } 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 { __private readonly int ownerId; __public class FindAllAccountsOwnedBy(int ownerId) __{ ____this.ownerId = ownerId __} __public ISet RunQuery(session) __{ ____ISet results; ____// Run query using whatever method I enjoy, and return results ____ __} } Running the query... ISet results = ISet.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.

Mark Hildreth commented on Thinking about repositories

Thu, 06 Dec 2007 23:17:30 GMT

Henning Eiben commented on Thinking about repositories

Thu, 06 Dec 2007 14:32:24 GMT

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 interface and a generic implementation GenericDAO : AbstractDAO, IDAO 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, IDAO, 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 myTaskDao = container.Resolve>();. In case I need some "special" features I can get TaskDAO myTaskDao = (TaskDAO)container.Resolve>();.

Mark Hildreth commented on Thinking about repositories

Wed, 05 Dec 2007 01:18:27 GMT

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, 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.

Ayende Rahien commented on Thinking about repositories

Tue, 04 Dec 2007 23:58:07 GMT

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.

Mark Hildreth commented on Thinking about repositories

Tue, 04 Dec 2007 23:54:56 GMT

I am in Joe's camp as well, although not sure of the implementation similarities. I have an IAccountsRepository inherit from IRepository, and an AccountsRepository inherit from Repository. I'm not sure the objection of "injecting IRepsitory 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.

Ayende Rahien commented on Thinking about repositories

Tue, 04 Dec 2007 18:32:06 GMT

Joe, the problem is not with this code: accountsRepository.FindAllInvactiveAccounts() The problem is with the associated implementation pattern, how you inject IRepository into the mix, for instance.

Joe commented on Thinking about repositories

Tue, 04 Dec 2007 17:25:16 GMT

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

Ayende Rahien commented on Thinking about repositories

Mon, 03 Dec 2007 07:36:11 GMT

I don't need linq for testing queries with DB. Check my post about specifications, that cover query object building, I think.

Nicholas Blumhardt commented on Thinking about repositories

Mon, 03 Dec 2007 07:33:37 GMT

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 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 :))

Ayende Rahien commented on Thinking about repositories

Sun, 02 Dec 2007 21:32:24 GMT

Nicholas, My repositories are almost always bound to a database, and I usually have a single implementation that handles everything there.

Nicholas Blumhardt commented on Thinking about repositories

Sun, 02 Dec 2007 21:28:50 GMT

@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: public interface IRepository { IQueryable Query(); void Add(object entity); void Remove(object entity); } 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.

Dave Foley commented on Thinking about repositories

Sun, 02 Dec 2007 20:48:52 GMT

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 Rahien commented on Thinking about repositories

Sun, 02 Dec 2007 11:10:09 GMT

> how is "AccountInactiveQuery" class implemented Probably by calling NH directly > how do you decouple NHibernate specifics from the IRepository? I don't need to, I have an expressive way to declare intent, that is what I am after. > How can I create on the client the same InactiveAccounts query specification and send it on the service for execution? 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.

aCoder commented on Thinking about repositories

Sun, 02 Dec 2007 02:03:23 GMT

Interesting, but how is "AccountInactiveQuery" class implemented and how do you decouple NHibernate specifics from the IRepository? 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.

Jeremy Gray commented on Thinking about repositories

Sat, 01 Dec 2007 21:21:30 GMT

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.

Ayende Rahien commented on Thinking about repositories

Sat, 01 Dec 2007 20:19:58 GMT

Jeremy, for me this is busywork without much value.

Ayende Rahien commented on Thinking about repositories

Sat, 01 Dec 2007 20:16:29 GMT

I am not sure that I follow you here, can you target the question a bit?

Jeremy Gray commented on Thinking about repositories

Sat, 01 Dec 2007 17:41:55 GMT

PS - they even recommended this for your code that externally exposes things like List, so the recommendation covered both your own generics types as well as MSFTs.

Jeremy Gray commented on Thinking about repositories

Sat, 01 Dec 2007 17:39:25 GMT

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, 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".

Matt commented on Thinking about repositories

Sat, 01 Dec 2007 17:19:04 GMT

Time to move to LINQ, my friend . . .

Reshef Mann commented on Thinking about repositories

Sat, 01 Dec 2007 14:14:05 GMT

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