CODEWITHSUNDEEP
asp.net-mvcrepository-pattern
Vlince
Vlince

Reputation: 6011

Repository Pattern: good or bad?

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:

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

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?

Upvotes: 3

Views: 4183

Answers (2)

Vlince
Vlince

Reputation: 6011

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.

Upvotes: 2

rouen
rouen

Reputation: 5124

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

Upvotes: 1

Related Questions