Ayende @ Rahien

Hi!
My name is Oren Eini
Founder of Hibernating Rhinos LTD and RavenDB.
You can reach me by phone or email:

ayende@ayende.com

+972 52-548-6969

, @ Q c

Posts: 18 | Comments: 79

filter by tags archive

Northwind Starter Kit ReviewFrom start to finishing–tracing a request

time to read 2 min | 243 words

This is a review of the Northwind Starter Kit project, this review revision 94815 from Dec 18 2011.

One of the things that I repeatedly call out is the forwarding type of architecture, a simple operation that is hidden away by a large number of abstractions that serves no real purpose.

Instead of a controller, let us look at a web service, just to make things slightly different. We have the following:

image

Okay, let us dig deeper:

image

I really like the fact that this repository actually have have FindById method, which this service promptly ignores in favor of using the IQueryable<Customer> implementation. If you want to know how that is implemented, just look (using the EF Code First repository implementations, the others are fairly similar):

image

 

All in all, the entire thing only serves to make things harder to understand and maintain.

Does anyone really think that this abstraction adds anything? What is the point?!

More posts in "Northwind Starter Kit Review" series:

  1. (26 Jan 2012) Conclusion
  2. (24 Jan 2012) That CQRS thing
  3. (23 Jan 2012) It is all about the services
  4. (20 Jan 2012) From start to finishing–tracing a request
  5. (18 Jan 2012) If you won’t respect the database, there will be pain
  6. (16 Jan 2012) Refactoring to an actual read model
  7. (13 Jan 2012) Data Access review thoughts
  8. (12 Jan 2012) The parents have eaten sour grapes, and the children’s teeth are set on edge
  9. (11 Jan 2012) Data Access and the essence of needless work, Part II
  10. (10 Jan 2012) Data Access and the essence of needless work, Part I

Comments

Rafal

The point is to increase the sales of your ORM profiler.

dotnetchris

IMO, the inherent failure here is the dependency this author has taken: ICustomerRepository. ICustomerRepository then implements a generic repository through inheritance which is epic fail. Generic repositories only provide value if they're produced through composition, not inheritance.

If this code showed:

public CustomerDetailWindowWorkerServices(IRepository customerRepository)

it would likely have far more value.

Completing repository implementations through inheritance is a gigantic anemic model problem. Leaving you with tons of useless classes like:

public CustomerRepository : GenericRepository { Public Customer FindById(string id) { return base.FindById(id) } }

Or other noise abstractions. Or even entirely empty classes

public CustomerRepository : GenericRepository { }

^ can't get any more anemic than that.

dotnetchris

IMO, the inherent failure here is the dependency this author has taken: ICustomerRepository. ICustomerRepository then implements a generic repository through inheritance which is epic fail. Generic repositories only provide value if they're produced through composition, not inheritance.

If this code showed:

public CustomerDetailWindowWorkerServices(IRepository<Customer> customerRepository)

it would likely have far more value.

Completing repository implementations through inheritance is a gigantic anemic model problem. Leaving you with tons of useless classes like:

public CustomerRepository : GenericRepository<Customer> { Public Customer FindById(string id) { return base.FindById(id) } }

Or other noise abstractions. Or even entirely empty classes

public CustomerRepository : GenericRepository<Customer> { }

^ can't get any more anemic than that.

First post had all my generics butchered, hopefully this one works.

Dan
Dan

@dotnetchris

I like your assertion that "Generic repositories only provide value if they're produced through composition, not inheritance." I've seen this done many times before and never been comfortable with it.

Would you propose that, instead of

CustomerRepository : GenericRepository{ }

Use something like:

CustomerRepository : ICustomerRepository { GenericRepository r {get;set;}

public Customer FindById(string id){ return r.FindById(id};

}

Or am I missing your point?

Lord spa

Despite many worthy views to the contrary I stick to lines of code at my favourite metric. It encourages the team to stop writing rubbish that others have to decipher and maintain, and to cut out layers of abstraction when they find them.

Stan

When your model is just a bag of properties and your application is just a CRUD you realy dont need repository. When you have complex model it is another story. It is a common case for commands when I should access one aggregate from other and than I prefer to inject repository into my entities instead of ISession. Just dont like to see NH use in a model. For queries I dont fetch model entities and than convert them to DTOs. I map DTOs to DB and directly use ISession in my query handlers.

Dmitry

@Stan

You are not supposed to have data access code (whether it is repositories or ADO .NET classes) inside entities. That is the whole point of the POCO concept.

Dan Turner

So I'm guessing your problem is with the Repository and not the Service?

Because in my opinion that service performs a clear purpose, it constructs a view Customer. Admittedly it seems as though the service and view could have been named better. I am also more than happy to see code like this NOT happen in the body of a controller action for all but the most simplest of scenarios.

Dan Turner

constructs a view of a Customer*

Phil

+1 to Dmitry. Having data access in your domain model is an absolute nightmare. Even though you can mock your DA interfaces it makes for way uglier tests.

Comment preview

Comments have been closed on this topic.

FUTURE POSTS

  1. The insidious cost of allocations - one day from now
  2. Buffer allocation strategies: A possible solution - 4 days from now
  3. Buffer allocation strategies: Explaining the solution - 5 days from now
  4. Buffer allocation strategies: Bad usage patterns - 6 days from now
  5. The useless text book algorithms - 7 days from now

And 1 more posts are pending...

There are posts all the way to Sep 11, 2015

RECENT SERIES

  1. Find the bug (5):
    20 Apr 2011 - Why do I get a Null Reference Exception?
  2. Production postmortem (10):
    03 Sep 2015 - The industry at large
  3. What is new in RavenDB 3.5 (7):
    12 Aug 2015 - Monitoring support
  4. Career planning (6):
    24 Jul 2015 - The immortal choices aren't
View all series

Syndication

Main feed Feed Stats
Comments feed   Comments Feed Stats