+972 52-548-6969
Northwind Starter Kit ReviewThe parents have eaten sour grapes, and the children’s teeth are set on edge
This is a review of the Northwind Starter Kit project, this review revision 94815 from Dec 18 2011.
In my previous posts, I focused mostly on the needlessness of the repositories implementation and why you want to avoid that (especially implementing it multiple times). In this post, I want to talk about other problems regarding the data access. In this case, the sudden urge to wash my hands that occurred when I saw this:
I mean, you are already using an OR/M. I don’t like the Repository implementation, but dropping down to SQL (and unparapetrized one at that) seems uncalled for.
By the way, the most logical reason for this to be done is to avoid mapping the Picture column to the category, since the OR/M in use here (EF) doesn’t support the notion of lazy properties.
Again, this is a problem when you are trying to use multiple OR/Ms, and that is neither required nor really useful.
Okay, enough about the low level data access details. On my next post I’ll deal with how those repositories are actually being used.
More posts in "Northwind Starter Kit Review" series:
- (26 Jan 2012) Conclusion
- (24 Jan 2012) That CQRS thing
- (23 Jan 2012) It is all about the services
- (20 Jan 2012) From start to finishing–tracing a request
- (18 Jan 2012) If you won’t respect the database, there will be pain
- (16 Jan 2012) Refactoring to an actual read model
- (13 Jan 2012) Data Access review thoughts
- (12 Jan 2012) The parents have eaten sour grapes, and the children’s teeth are set on edge
- (11 Jan 2012) Data Access and the essence of needless work, Part II
- (10 Jan 2012) Data Access and the essence of needless work, Part I
Comments
Bestides this, I believe there's another thing that one should avoid at this level of abstraction: System.Drawing.Image as this has got to do with GDI stuff and this namespace doesn't make sense in the web world. Also, in WPF (unless you NEED to do some GDI stuff), you will not need System.Drawing at all and you might want to opt for something else just to display images on the screens... I haven't seen the place where this repository is used, but I guess it's only Winforms so perhaps a better name for this would be WinformsCustomerRepository? :)
"By the way, the most logical reason for this to be done is to avoid mapping the Picture column to the category, since the OR/M in use here (EF) doesn’t support the notion of lazy properties."
Even that would be no reason to do it like that, why not use a binarydocument class that would contain the picture, this would allow lazy loading on the many-to-one side from category.
@Ayende: Is "unparapetrized" a shortcut for unparameterized? ;)
@Ayende: Is "unparapetrized" a shortcut for unparameterized? ;)
I would only return the byte array. But hey how loosy do you want to go. I would say it's a nice readable and maintainable code and super fast. Or maybe cache (in memory) the categories and pick it from memory instead of DB.
@ayende,
these posts are in now way useful!!
you are not saying how to do it right, just pointing out how wrong this approach is.
how is this helpful?
@Ignacio
If you have to choose between 3 types of bread and I tell you that mouldy bread is not tasty was that helpful for you?
@ Nikolai
thats not the case here at all...
ayende here is just telling me that mouldy bread is not good... hes not even mentioning other kinds of bread.
Well, actually he is. good bread = use the orm directly, because it is actually a repository. If we are talking about NHibernate use ISession/ISession instead of leaky IRepository abstraction. Regards
@Ignacio
There's some pretty good bread here: https://github.com/ayende/Alexandria
Returning Image from repository seems so wrong.
Ayende I don't know how lazy properties work in NHibernate but you can just create another object (Image) and add association from Category to Image and then you have lazy loading of image.
If we have ActiveContext why not call ExecuteStoreCommand instead of creating command by hand? Also the reader is not disposed.
Kids in the back, people. http://herdingcode.com/wp-content/uploads/HerdingCode-0131-Chris-Williams-and-Matthew-Podwysocki-on-the-Javascript-community.mp3
@Ignacio
I hate to say it, but you must be new. This is what this guy does all the time. It's troll bait to get people mad so they talk about him. Of course he's an a**. Everyone already knows that.
well technically, you can lazy load properties in EF. You have to split the table into two entities.
@Bek - That would be changing your design to work around the limitations of EF rather than not having to worry about limitations to begin with.
Yep use the context or session directly in your controller, lets just move back to code behinds while we are at it.
@Phillip Well, it's 6 eggs one way, or half a dozen the other. I've seen plenty of problems with NHibernate implementations where the issues boil down to bad assumptions about what should be, or shouldn't be lazy loaded. Several were set up to either lazy load everything or nothing.
Having a design that explicitly designates an entity that's intended to be loaded separately may be a change in design, but not a bad change in design.
@Steve - I'm not saying NHibernate is perfect or that NH is better than EF. All I mean is we shouldn't have to change our implementation just because something is not supported by a framework.
I demand next article in this series. The plot is getting interesting and I'm sinking into it
@Phillip And I think your comment is the linchpin to Ayende's entire argument. If it turns out that the ORM I've chosen has limitations and I've abstracted it away into a repository, I CAN'T work around it (i.e. ship code and make my client happy). Since I have this lovely abstractions, I say EF sucks and switch to NHibernate only to discover it has limitations in other areas. Ooops.
You really will need to be able to optimize queries for the situation at hand. The only code that knows that situation is the consuming code.
If you abstract away the ORM, you can't use it's features to optimize. If you expose it's features, you just leaked your ORM's implementation details through your abstractions and now you can't switch ORMs anymore.
So you might as well just use your ORM from the beginning and ship code.
EF or NHibernate both let us switch between many database engines. Once we work out all the issues of that abstraction for one ORM, what value do we get from switching our ORM?
Comment preview