The authors probably refer to themselves as Software Craftsmen too ;-)

Heh, that's exactly what I used to do in my application until some time ago, when I realized that I have almost 50 classes of this kind, with almost but not quite identical code (which is an excellent incubator for bugs).

When I started out, I thought that using repositories was the only way to go if I wanted to be able to do mocking and integration tests, but the tests were very fragile, as I had to mock every single call to the database.

I've since moved to a Query object pattern, and it works great. The code is much more minimal, I can define common conditions for types of queries, and I can really test the code by using in-memory sqlite database.

This kind of coding is perpetuated by so-called best-practice showcases, like 'Who Can Help Me' ( http://github.com/jongeorge1/Who-Can-Help-Me). Some time ago, when I looked through it, I thought it is so cool and elegant. I adopted some patterns I've seen there in my code.... and after working for a week or two this way, I began to hit walls every step I took.

WCHM project looks a bit better now, but just look at some older branch: github.com/jongeorge1/Who-Can-Help-Me/blob/N2CMS - awful unnecessary repository classes, the same "specification" pattern as found in Whiteboard Chat application, etc.

I know that writing infrastructure code and inventing clever patterns is sometimes more pleasing than implementing boring features - and maybe that's why we have so many over-engineered projects that are pain in the ass to maintain. It's really encouraging that there are developers who are not afraid to say that simple is better.

Eh Eh Eh. Ayende, you're not afraid to say what some of us have always thought.

Trying to figure out what happens in those projects (Who-Can-Help-Me is one of those, yes) is a real pain in the ass.

I've felt dumb in the past but than I had realized that the guys are trying to show off just for the pure pleasure!

There's no need for all that c**p in a project.

I use something similar to this myself:

public class CustomerRepository : NhRepository <customer, ICustomerRepository

Although the classes that support this aren't anywhere near as complicated as above.

In my implementation there's an IRepository <t with methods for FromId(id); Save(), Delete(), and IQueryable <t Query(), and then the Nhibernate implementation of IRepository (NhRepository <t). The benefit I've found is that your implementation for Save, Delete, FromId etc. are defined just once and work for all entities (DRY is important isn't it?), so the code to save a customer is this:

_customerRepository.Save(customer);

instead of this:

ISession session = SessionManager.GetSession();

using(ITransaction trans = session.BeginTransaction())

{

session.SaveOrUpdate(customer);

trans.Commit();

}

I much prefer the former. And if you want to change the isolation level, or have transactions span multiple repository calls, then by all means overload the Save() method to allow this, but in 90% of cases I don't need this.

I used to be obsessed with completely abstracting NHibernate anyway in my repository, but now I don't see any harm in returning NHibernate types from your repository (such as ITransaction), and have a dependency on the NHibernate libraries. Basically, I still see value in the Repository pattern as long as you don't take it too far and over engineer, or obsess/worry over changing data access technologies. Also for me, Repositories provide a clear/semantic separation of your Aggregates if you're using DDD (one Repository per Aggregate Root etc.)

OK, so the above code sample should be:

public class CustomerRepository : NhRepository<Customer>, ICustomerRepository

(doesn't like HTML angle brackets)

I recently got hold of the book asp.net design patterns by Scott Millet who has the case study from the book at http://aspnetdesignpatterns.codeplex.com/

I wonder what do you think of code like this, as young developers like me think of it as a must have reference (code I need to practice many times to learn from it). It does seem to cover repositories as Dominic has described above but the repository itself can be replaced by NHibernate or EF

EmailTemplateLookup is then implicitly convertible to int? This fails on so many levels (pun intended).

Now, instead of backlashing the whole thing to inlining SQL and using a DataReader (I'm surprised noone has proposed yet for read models) it would be nice when people turn on their head, try to write readable code and maybe have a few OO principles back in their mind and then we're getting there.

And no, this surely has nothing to do with craftmanship.

I have to admit, we have developed a data access layer that looks very similar to this on a project that our team is working on. We thought it was a clever way to use DRY for basic CRUD operations, but still providing the flexibility to extend it as needed for more complicated query scenarios. I definitely see how the abstraction can be a little hard to digest at first glance. At the same time, once you know how to plug into this "framework" you get a lot basic functionality for free. I don't think the trivial implementation bothers me so much, however....the repetition of the trivial code does give me a little pause, as it may be an indication of a problem with the design. I guess I am very curious on some more ideas around the example above. What are some of the issues with this? What are some better options?

This is very similar to the code that you can find in the CodeCampServer project. To me, the CCS project showcases the kind of overly complex architecture that is so prevalent in the .Net space, yet, it is supposed to be "best practice". I always found it amusing to run this app (it's dead simple), and then look at the mammoth code base. There is something very wrong with that picture.

We also ended up doing something similar, but the main reason was to make it easier to unit test anything involving DB access. So we had a base repository like this:
  
  
class RepositoryBase
<t { public IQueryable
<t Queryable { get { return Session.Query
<t(); } }
  
  
then extend, perhaps without even adding any methods:
  
  
class MyRepo : RepositoryBase
<myentity { }
  
  
We can then mock out the Queryable property in our unit tests for anything that queries the DB (much more easily that mocking ISession, as the Query
<t method is implemented as an extension method.) This removes the need for the in-memory DB (which is still a lot slower than using objects directly, by a significant margin.)
  
  
I'd definitely be grateful if anyone can point me to a better technique.
  
>

class RepositoryBase>T< { public IQueryable>T< Queryable { get { return Session.Query>T<(); } }

class MyRepo : RepositoryBase>MyEntity< { }

Well, you get the idea...

I have always found this particular style of code to be odd. The main brunt of it is that they are defining abstractions for the sake of technology and ego satisfaction - but if you're going to try to do that (and more importantly you believe you're very likely to have to switch technologies midstream), you should probably only have one single abstraction, instead of a hundred senseless layers of them. If you don't have any reason to believe that you need to support switching technologies, then 99% of the time, you're better off using the technology directly in order to really get the power behind it.

I typically rely on abstractions for business processes instead. The reason for this is that the business processes are by far the most likely to change, even if only just a little bit. That little bit, however, can be huge in the code, even if conceptually small for the business. I think about it like this: When have you ever had to wholesale switch out the use of SQL Server for an Oracle DB vs add a single field to a DB schema to represent some obscure business-related entity? I suppose for some people, the former is very high, but at least for me, the latter is by far the most prevalent.

I agree with Kyle. Abstract out what is likely to change....not what is likely to stay the same. The latter is just ego gratification and a false sense of having "good" architecture.

I think the moral of the story, if you want to use the Repository pattern, use it with a healthy dose of pragmatism.

I measure the merit of an abstraction primarily by how much the base class code is reused. Yes I agree this looks like a ton of abstractions, but lets consider the context:

1. This is unwieldy for a small simple database, but are we dealing with 100+ tables/entities of a similar nature?

2. Abstracting away your persistence technology looks like over-engineering, but is there a chance that one or more of these entities will outgrow/oversize its sql database persistence medium, and we will swap it out with a document-DB down the road?

In other words, do these abstractions yield benefit on a larger scale?

I use the "templated" repository as a pattern as a rule of thumb in quite a few non-trivial database projects. Yes, it is indeed a lot of boiler plate code, but often a cut-and dry mechanical consistent convention serves better than going for the elegant abstraction.

But then again, I'm the kind of guy that often opts for BLToolkit

Repositories are always evil if you try implement them correctly, like the guys who wrote Sharp Commerce. If you want to have a fully interchangable data-access-layer, then you will need this kind of abstraction. I just wonder, if someone has ever done this in a real-life project, without modifying at least the business-layer. Anyone?

I think code like this comes about for 2 reasons.

1. the maturity of the .net community. We (as a whole) are still in the phase of learning what good design is. When we see a pattern, we use it. usually without asking "if" or "why".

2. The myth of abstracting 3rd party persistence. whether it's swapping EF & NH or RDBMS for Doc DB. There is a belief that an abstraction will make that transition less painful. Along that same line of though, I think some devs believe there is value in "testing" queries without actually hitting a database.

To be fair, we should probably point out that most of this architecture comes from S#arpArchitecture ( http://www.sharparchitecture.net/), a project that SharpCommerce references (WhoCanHelpMe is based on the same framework). I have a pet project based on S#arpArchitecture as well, and I did something similar as SharpCommerce did by creating their own linq repository over the base repository.

As I get more familiar with NHibernate, I see how this design adds needless complexity and I've already started thinking about how I can redesign my project to get away from S#arp and all of its trappings, and use NH more directly. However it's important to note that using NHibernate can be intimidating to a NH-newb, and frameworks like S#arp are attractive because they establish a pattern that at least gives you the (incorrect, in some cases) feeling that you're using it "the right way".

I would tend to agree that designing a solution purely for the sake of being able to swap technologies midstream is probably not always a great idea. I agree with Pete in that you have to consider the context. We used a templated repository pattern for the following reasons:

1. Depend on abstractions, not concrete implementations - this provides a nice pattern for defining new interfaces for querying your domain objects, while providing basic crud in the base class.

2. Team Makeup - It also provides a consistent way of doing data access regardless of your skill level and understanding of OO. It helps keep data access logic from polluting layers higher in the application stack.

3. Simplifies Client Code - While the abstraction does increase complexity, client code is much cleaner in that it just sends a message to the repository for whatever operation it needs performed.

Also, let's go with the assumption that no one here is writing code for ego satisfaction. Personally, I'd like to hear constructive criticism.

I'm seeing more and more value into introducing a read layer/boundary into my applications. One abstraction. For example, ICustomerQueries. On more than one occassion, I've had to switch from LLBL querying to LinqToSQL for things like unions that are not supported by the provider. Also, needing to use join hints is as simple as creating a database view, which can sit nicely behind this abstraction. No mandated base classes whatsoever (however, yields high composability scenarios).

Totally agree with Kyle. Business processes are generally much more fluid and mutable than the underlying database (etc.) technologies.

It's important to remember that patterns and principles are there to help us make solid, predictable choices in our projects. Like any set of principles, it's easy to get carried away in their application and find yourself burdened by an entirely new set of additional problems.

In my experience, I've gained much more bang for the buck trying to keep a process chain loosely coupled than throwing a lot of energy at assuming a wholesale alteration of data storage is coming. Of course, this is all in relation to the specifics of the project and my workplace, but that's the point. Sometimes I have only 2 days to author a data model and get a front end up and running. At those times I'm grateful for out of the box functionality like what I find in EF. I'll then use whatever's helpful from something like MVVM to keep the interface easy to change.

That makes me feel better. I shirked away from trying Sharp Architecture when it was gaining momentum a few years ago, for pretty much the same reason. All that "stuff" weighing on my brain.

I think it had some reasonable selling points, but I'm too lazy to get my head around stuff and write lots of code :)

@Dominic - there are several reasons to prefer writing out GetSession, BeginTransaction, etc.. But the biggest one is that hiding them inside the repository puts session and transaction management in the wrong place; callers know best how to scope them.

It would be interesting to see your take on NerdDinner (at CodePlex), that project is not NHybernate (it's LINQ2SQL and EF depending on version). But that project is actually used (in books) to teach people about Microsoft MVC and QRM integration with it.

Oren - good to see you post your comments on this as well as see the others opinions. The Sharp Architecture team is in the middle of re-writing a lot of this code, as well as removing quite a bit of the indirection that is found in the repository area.

Part of the problems we saw was that we had IRepositoryWithTypeId <t,> , then IRepository <t that implemented the former. Then there were 4 concrete implementations on top of these. This is now going away - which ironically occurred about the same time as your original posting. I am a fan of a generic repository interface for the simple fact I can leverage it to place different persistence mediums behind it, in this case NHibernate and RavenDb (new in 2.0). But what we found is that the concrete implementation for IRepositoryWithTypeId, RepositoryWithTypeId, was completely specific to Nhibernate, even though there was a NHibernateRepositoryWithTypeId as well.

I agree with a lot of what is being said, and once again thank the commenter's for speaking up. If you have any other feedback, negative or positive, I would love to hear it and potentially learn from it.

Thanks

Alec Whittington

Sharp Architecture

Er, just to clarify what I wrote above - the alternative to:

_customerRepository.Save(customer);

Isn't even to write out GetSession, BeginTransaction for every operation. Instead, you can push session and transaction management out (for example into request event handlers, action filters, etc..), so that ultimately you're deciding between the code above and:

session.SaveOrUpdate(customer);

@Jeff I like to begin and commit my transactions in the application layer where i have various handlers for each business process/command.

@Jeff Sternal

I disagree, putting them in the Repository puts them in the right place for 90% of times when you just want to save something and don't really need transaction management. For those times when you need more fine grained control of Transactions, you can still have the caller get an instance of ITransaction and manage it.

@Jeff Sternal and @Ayende Rahien

I should point out that I'm using the Session per Request pattern (it's a web application), which is recommended practice according to the official NHibernate docs (they even provide some sample code for it). I know it smacks of utility class with SessionManager.GetSession(), it was a simplified example, my code doesn't quite look like that.

Also, what do you mean by "That is the responsibility of the context,not your code"? Do you mean as in request context (for a web app) e.g. Start transaction at start of web request, commit at end?

@Jeff " Start transaction at start of web request, commit at end?"

I've never bought into this for managing transactions. Surely it is driven by context, but that's a little long lived for me.

I am implementing a new project, and originally I had a light-weight repository for querying only. This has since been replace with a pattern which looks very much like this and I can see that there will be an explosion of repositories and worse, repositories which look very similar containing just one find by id method. This anti-pattern is definitely not the way to go, but I'm dead against using the object context directly, I'd like to keep track of what queries I am executing. Doesn't have to be complicated though.

Ahh I see, I didnt realize we were actually critiquing SharpArchitecture here! I knew the design above looked familiar!

The origin of the design above can be traced back to "NHibernate best practices" article on codeproject By Billy Mccafferty. ( www.codeproject.com/.../...rnateBestPractices.aspx) Billy's article was largely accepted as "this is how you do NHibernate with ASP.NET" for many years so its funny to me seeing so many people in this forum taking digs at that design today.

@Ayende I cannot disagree the "templated generic repository" approach above can make you paint yourself into a corner when using stateful session-aware persistence like EF or NH. ActiveRecord always worked for me in these scenarios

@pete w

To be fair, the stuff outlined in Billy's original article have very little to do with the generic repository stuff that's debated now. SharpArchitecture has quite a few nice concepts and I believe it started out as a good intentioned "reduce friction" project, but architecture took over and it's now becoming less and less relevant as a good framework.

I agree about ActiveRecord. When it works well it's productivity heaven. The "persistence ignorance" mantra managed to almost kill it, but it is indeed a working approach in many scenarios.

humm

Learning how not do something is very cool and important... but where is the counter part? The "how to do things"?

Nice video about the best practices subject exposureroom.com/.../1fc0a77839354170a009e6bcaf...

My current approach is to load aggregate root from repository, modify and save it, applying a nestable Transaction attribute on the calling method. For views, criteria API or an sql-query do an excellent job loading view models directly from db using an imported class, I use sharparchitecture on this project without any friction and the benefit it provides are great (I am a bit biased though), but I rarely end up with repository classes like EmailTemplateRepository.

Regarding the specification pattern used in WCHM, I have tried it once with the NH2.1 unofficial nhibernate linq provider, and was limited by it's lack of functionality, but I quite liked the idea of chaining specifications and it made mocking the repository much easier.

Looking forward to reading the rest of the posts.

Hmmm... When i started taking photos and bought my first camera, the first I did was to put an uv filter to "protect" my lens...

I used it until a good friend and photographer of mine asked me: why would you spend so much money to buy a first class lens, just to put some crappy glass on top of it...

Smart people put so much effort to get rid of SQL, only to end up like this.

Why go all this trouble to come up with something as good as hibernate just to shadow it with patterns like this?

What is a pattern anyway???

Here is my implementation of repository. There is only one interface and one implementation class. Both, repository and find methods work with IQueryable so Fetch, Cachable, Skip, Take, OrderBy, additional Where clauses and any other LINQ/Nhibernate extension methods can be applied on the query level.

Specifications are only used in cases where the Where lamdba expression would be difficult to understand. It could be customer categorization, accounting rules, etc.

public interface IRepository<T, in TKey> : IQueryable<T>

where T : Entity<TKey>

{

T this[TKey id] { get; }


T Get(TKey id);


IQueryable<T> Find(ISpecification<T> specification);


object Add(T entity);

void Remove(T entity);

void RemoveById(TKey id);


void Refresh(T entity);

}

The indexer is used for lazy loading proxies - a convention from the old system.

In the end all the discussion is for a simple reason...some years ago you and others told us to do things with Repositories and abstraction layers etc.

Nowadays all of this is suddenly false. You have to admit that this is a little confusing.

So my conclusion is...i do it in a way that seems to be right for me and my projects. So for instance I like the idea of abstracting NH away from the rest of my code. This is not only to have the ability to change the OR/M in the future but to have on simple an clear API every team developer can simply understand and use and I do not see why this should be bad. Using NH directly in view's or something else remembers me of times ten years ago. But who cares, it is a free world and everyone can do what he wants.

But you're right, there is no need to overcomplicate things like in the example above.

@Ayende Rahien

Since you are so anti-repository, could you suggest what we should do instead (maybe a blog post on it). For instance, are you advocating that we should do this (for an ASP.NET MVC app):

public ActionResult ListProducts(string search)

{

ISession session = // Get session somehow

ITransaction tx = session.BeginTransaction();


IQuery query = session.CreateQuery("select p from Product as p where p.Name like :name");

query.SetString("name", "%" + search + "%");


IList

<product products = query.List <product();

tx.Commit();

session.Close();


return View(products);

}

If not, then what?

This is how I would write out the above with Repositories:

public ActionResult ListProducts(string search)

{

IList

<product products = _productRepository.SeachProducts(string search);

return View(products);

}

Seems much cleaner to me, data access code is neatly encapsulated away, doesn't violate SoC or DRY, what's wrong with it? How would you approach it differently?

To quote E. Dijlstra: "The purpose of abstraction is not to be vague, but to create a new semantic level in which one can be absolutely precise"

@Ayende,

An indexer is just a convention developers are used to. I forgot to remove it (since it is irrelevant to the discussion) that's why I created another post. It would be a Load or a Get overload if I was designing a public API.

I am not sure what you mean by "depending on NH API everywhere else"? The unit-of-work interface contains SubmitChanges(), Dispose(), Attach(), Detach(), Merge() methods and a T4 generated partial class with repository properties such as IRepository<Company> Companies { get; }. A controller subclass is responsible for starting, committing and disposing the unit-of-work.

Granted there are queries that had to be done using Criteria API. Those are abstracted in classes that use the underlying NHibnerate session through DI. Hopefully as NHibernate evolves most of them would be possible to do with LINQ.

@Dominic,

Your repository object looks more like a DAO/service.

A repository (in DDD) is supposed to be collection-like abstraction of aggregate root entities. That means you are not supposed to have custom implementations and methods for specific entity types. It should be thought more like IEnumerable<TEntity>

I do not think Ayende is against abstracting data access code in DAO classes that use NHibernate API through dependency injection.

@Gunteman - Care to explain your position further? Sharp Architecture has actually changed little since the first release by Billy. Major changes are underway now, but every project evolves over time, do they not?

Have encountered the useless layer abstraction madness in my last company - so much that I had to leave and emigrate to another country in disgust :)

Surprised how easily the .NET community give enough ammo to every developer to shoot themselves in the foot. Almost every other guy I have interacted wants to live and breathe fluent lambda syntax and is trying to get fancy with expression trees and fancy abstraction layers.

Wonder how many developers in say the Python world want to write a meta class? I think we generally tend to leave meta class programming to the people who know what they are doing, and are happy building out features.

Six months into Django quite happy with the queryset api and not yet felt the urge to wrap it in a repository.

On a final note, on the earlier post about not having common libraries - I'd rather have all the crap I've written over the years (or whatever that is my colleague is snickering at) in a single assembly than 10 different ones.

And by the way the abstraction guys got promoted, and I had to search for a job - maybe abstractions are there for a reason :)

Dmitry's repo is close to what I use. Basically IRepository <t is just IQueryable <t plus InsertOnSubmit and DeleteOnSubmit. You only need one concrete class (like LinqRepository <tentity,> ).

It's useful for a few reasons:

1. if you have true repeated / shared query logic, you can model that as extension methods from IQueryable <t to IQueryable <t (specs are now easy to test with List or whatever)

2. you can use the IRepo as IQueryable in your controllers if you don't need the abstraction and you are fairly close to your persistence 'metal'

3. you can have concrete classes for other persistence strategies as long as they provide IQueryable. My scenario here was a mainframe integration product that spoke Entity Framework. We could have our main persistence done with linqtosql or nh but use a consistent repository model to talk to the mainframe integration piece.

Dude, with this one you made me laugh for half an hour.

How is call this? over-f.... architecture?

:-)

I love the focus on simplicity at the moment and cutting down all the architecture crap and abstraction madness. Please write more stuff along the lines of this - you are a respected individual and have some weight in a lot of companies. You are saving us from architecture tyranny.

@Dev ... Oh also without wishing to piss anyone off, Scott Millet's book (Professional ASP.Net Design Patterns) was a hopeful purchase by myself. It's actually full of leaky, messy abstractions, epic violations of SOLID and all sorts of incongruent mess. I don't like it at all.