Repository Pattern: good or bad?

Question

After reading on the benefits (and using) the Repository Design pattern within an ASP.NET MVC 3.0 project, I’ve come across a certain issue which got me puzzled and started to question the benefits of the pattern. Perhaps someone could help me clarify this.

I have the following 3 TABLES:

-User (parent)
-UserLead (child of User)
-UserLeadNotes (child of UserLead)

Roughly, the tables and their relationships are designed like this:

User [Id]
UserLead [Id, UserId (Foreign Key)]
UserLeadNotes [Id, UserLeadId (Foreign Key)]

I have a BaseRepository which include the basic CRUD methods. (IEnumerable<TEntity> Get(), TEntity GetByID(), Insert(), Delete(), etc…)

With that in place, I also have a Service Layer which calls the appropriate repositories when and if needed.

In a nutshell, the application displays a UserLead and allows a user to add/remove notes to that UserLead (which are stored/deleted in the UserLeadNotes table). Adding/removing notes are done via Ajax calls. The removing of a note passes along the “id” of the note I wish to delete.

Now…before I actually delete the note, I need to make sure that the note truly belongs to the current logged in user (User.Identity.Name aka UserId)

Considering my method deleteUserNote(int noteId) only receives a “noteId” parameter AND considering that my UserLeadNotes TABLE does not have a foreign key on UserId, what I need to do, is add an

.Include(“UserLead’)

inside my _userLeadNoteRepository to have something like:

UserLeadNote userLeadNote = _userLeadNoteRepository
                .AllIncluding(p => p.UserLead)
                .FirstOrDefault(p => p.UserLeadNoteID.Equals(noteId) 
                   && p.UserLead.UserID.Equals(User.Identity.Name));

Then I would delete the UserLeadNote object that I just found:

_userLeadNoteRepository.Delete(userLeadNote);

Questions:

Following the above approach, this would force me to a) create a Repository for my UserLeadNotes and b) because of the .Include() the userLeadNote object found would have a property called UserLead holding all the fields defined in the UserLead table.

If my UserLead table happens to have 14 fields and 10 of these fields are ntext, that means I would be loading/holding too much information for nothing!

If I was to use, instead of a Lambda expression, a Linq statement bypassing the repository and directly use my Context:

using(MyContext db = new MyContext ())
            {
                var query =
                    from uln in db.UserLeadNote
                    join ul in db.UserLead on uln.UserLeadID equals ul.UserLeadID
                    where uln.UserLeadNoteID == userLeadNoteID
                        && ul.UserID == User.Identity.Name
                    select uln;

                var userLeadNote = query.FirstOrDefault();

                db.UserLeadNote.Remove(userLeadNote);

                db.SaveChanges();
            }

Wouldn’t this approach be more efficient as it would not bring back those unnecessary fields from the UserLead table? (Those 10 ntext field for example).

Unless I’m not using the Repository Pattern correctly, or perhaps this is an Entity Framework issue, I’d like to know what anyone thinks!

Keep in mind, that I am aware of the benefits of having a Repository Pattern in place but curious if this is some kind of issue people are having or not. And if so, have they decided to drop the Pattern and directly use their Context using Linq statements.

What’s the lesser of two evils?

I used the pattern on a few projects, but dropped it in favor of going directly to the context. If you continue to use the pattern, you should read up on **Aggregate Roots**. You don't want a repo for every table/entity, just your core ones. I typically reserve full blown DDD for complex business logic/relationships. http://devlicio.us/blogs/casey/archive/2009/02/16/ddd-aggregates-and-aggregate-roots.aspx — Ryan, Dec 08 '11 at 18:28
I now better understand that my UserLead and UserLeadNotes table would be considered as an Aggregate. So my UserLead table would be considered as the Aggregate Root. This means I should only have a single Repository for that Aggregated Root table and navigate from that Root to my children(s). With that in mind, how would having my single UserLead Repository allow me to navigate to my child and obtain a FirstOrDefault() child object without loading the parent’s additional fields? Perhaps you are right; I should drop the Repository Pattern and directly invoke/use my Context from my Service Layer — Vlince, Dec 09 '11 at 13:51

score 2 · Accepted Answer · answered Dec 09 '11 at 15:25

In conclusion,

After reading Ryan’s and Rouen’s posts, It appears that creating one Repository per Entity might not be the appropriate approach. If I have a child (or child of child) entities, I should be instead creating a single Parent Repository and navigate my way trough the child (or child of child).

The problem I was confronted with is that when trying to delete a child of a child entity (that did not have the Parent’s foreign key) I had to .Include(“”) those other “upper” tables. (In my case it was only one table, the Parent but one could easily have to delete a child of a child of a child and would have to Include() many “parents/upper” tables).

In the end, Including the Parent table created a property which held all the fields of the Parent and in my case, that was considered “getting” too much information for my simple task at hand.

This so called problem, could be avoided if I was to drop the usage of the Repository Pattern and instead, directly use the Context.

For some, this might not be a problem (or they simply don’t know what’s happening) and for others, having the Pattern in place outweighs the Cons it may have.

I haven’t decided yet on completely dropping the Repository Pattern since I like what it brings to the table.

Thanks to all.

Glad I could help. What really made me think about my over use of the repository pattern was a series of posts on Ayende's blog. http://ayende.com/blog/tags/design Start reading at "The wages of sin" and then probably 12 posts in a row (reading down the page) are absolute gold. — Ryan, Dec 13 '11 at 00:15
@Vlince, I am having the same "getting too much information for my simple task at hand" with multiple Include calls. Thinking about repositories but not sure: http://stackoverflow.com/questions/17529796/what-is-the-best-practice-for-multiple-include-method-calls-in-entity-framework — anar khalilov, Jul 30 '13 at 13:13

score 1 · Answer 2 · edited May 23 '17 at 12:03

1

Try to look here :

asp.net mvc page is not showing properties from associated objects

there is my answer with explanation/opinion about this kind of misuse of generic repository pattern

edited May 23 '17 at 12:03

Community

1
1

answered Dec 08 '11 at 19:50

rouen

5,003
2
25
48

Thanks for the post! So in the end, like mentioned in Ryan’s comment, I should probably drop the Repository Pattern and merge my queries directly in my Service Layer directly using my Context. This would increase performance and return me exactly what I want without extra data/fields. The tradeoff in dropping the pattern is that now my Service Layer would be tightly coupled to my database. Oh well…as long as I’m aware of that. Thanks – Vlince Dec 09 '11 at 14:00

Repository Pattern: good or bad?

2 Answers2