This is a review of the Northwind Starter Kit project, this review revision 94815 from Dec 18 2011.
Update: Andrea, the project’s leader has posted a reply to this series of posts.
Yes, this is another repositories are evil if you are using an OR/M post.
That is probably going to cause some reaction, so I am going to back this up with code from this NSK project. Let us talk about repositories, in particular. Let us see what we have here:
Okaay…
Now here are a few problems that I have with this:
- There is no value gained by introducing this abstraction. You aren’t adding any capability what so ever.
- In fact, since all OR/Ms provide an abstraction that isn’t dependent on type, creating IRepository<T> and things like ICustomerRepository is just making things more complicated.
- There are going to be changes in behavior between different repositories implementations that will break your code.
Let us see what we actually have as a result. This is the Entity Framework POCO implementation:
You can probably guess how the rest of it is actually implemented. Yes, we have a LOT of code that is dedicated solely for this sort of forwarding operations.
And then we have the actual implementation of the delete:
Just to remind you, here is the NHibernate implementation of the same function:
Leaving aside the atrocious error handling code, the EF POCO version will do an immediate delete. The NHibernate version will wait for the transaction to be committed.
And don’t worry, I do remember the error handling. This is simply wrong.
And then we have implementations such as this:
This is for the Entity Framework Code First implementation. There is a message here that is coming to me loud and clear. This code wants to be deleted. It is neglected and abused and doesn’t serve any purpose in life except gobble up pieces of valuable disk space that could be filled with the much more valuable result of reading from/dev/random.