ASP.NET MVC using Repository pattern

Question

I am just trying to use ASP.NET MVC using repository pattern. Am I doing something wrong here?

Model - Contains the model

public class Contact
{
    public int Id { get; set; }
    public string FirstName { get; set; }
    public string LastName { get; set; }
    public string EmailAddress { get; set; }
}

Repository interface - Interface

public interface IContactRepository
{
    void Create(Contact contact);
    void Delete(int contactId);
    void Save(Contact contact);
    Contact Retrieve(int contactId);
    IEnumerable<Contact> Select();
}

Repository class

public class ContactRepository : IContactRepository
{
    private AddressBookDb addressBookdb = new AddressBookDb();

    public void Create(Contact contact)
    {
        addressBookdb.Contacts.Add(contact);
        addressBookdb.SaveChanges();
    }

    public void Save(Contact contact)
    {
        Delete(contact.Id);
        Create(contact);
    }

    public void Delete(int contactId)
    {
        Contact contact = Retrieve(contactId);
        if (contact != null)
        {
            addressBookdb.Contacts.Remove(contact);
            addressBookdb.SaveChanges();
        }
    }

    public Contact Retrieve(int contactId)
    {
        return addressBookdb.Contacts.FirstOrDefault<Contact>(c => c.Id == contactId);
    }

    public IEnumerable<Models.Contact> Select()
    {
        return addressBookdb.Contacts.ToList<Contact>();
    }
}

Controller class

public class ContactController : Controller
{
    IContactRepository contactRepository = null;
    public ContactController() : this (new ContactRepository())
    {

    }

    public ContactController(IContactRepository contactRepository)
    {
        this.contactRepository = contactRepository;
    }

    [HttpPost]
    public ActionResult Edit(Contact contact)
    {
        if (ModelState.IsValid)
        {
            contactRepository.Save(contact);
            return RedirectToAction("Index");
        }
        return View(contact);
    }
}

Answer 1

I have a very good solution for you. See the below two blog posts:

Generic Repository Pattern - Entity Framework, ASP.NET MVC and Unit Testing Triangle

How to Work With Generic Repositories on ASP.NET MVC and Unit Testing Them By Mocking

What I can suggest more is this:

Implementing IDisposable:

Implement IDisposable on your repository and dispose them on your controller by overriding the Controller.Dispose method. Sample:

IContactRepository:

public interface IContactRepository : IDisposable {

    //same as above
}

Add the below code to your ContactRepository class:

private bool disposed = false;

protected virtual void Dispose(bool disposing) {

    if (!this.disposed)
        if (disposing)
            addressBookdb.Dispose();

    this.disposed = true;
}

public void Dispose() {

    Dispose(true);
    GC.SuppressFinalize(this);
}

On your controller, add this code:

protected override void Dispose(bool disposing) {

    contactRepository.Dispose();

    base.Dispose(disposing);
}

Answer 2

Couple of things I'd change:

1. Make save work out whether it needs to create or modify itself.

2. Don't delete and re-create in the same. It will mess with relational data as Renato said.

3. In the Retrieve method return SingleOrDefault - Only one contact record should have a unique id. Then let the controller check for null instead of a try/catch which you'd need around a Single call as this would throw an exception if no record was returned. FirstOrDefault makes it look like you're not confident there will be only one record; also depending on ordering it could return a different record depending on conditions.

4. You could make you save method return a bool value to stop the db side of things throwing an error and crashing the site. You could then do the following:

   [HttpPost]
   public ActionResult Edit(Contact contact)
   {
       if ((ModelState.IsValid) && (contactRepository.Save(contact)))
       {
           return RedirectToAction("Index");
       }
       return View(contact);
   }
   

Answer 3

I may come back and fill this answer out more, time permitting. But I would do as others here suggest re a Generic repository... As a starting point on this take a look at this 6 min video.

A little difficult to understand his accent but the code speaks for itself and for such a short example it really does get through a lot.

The reason I recommend a generic repository with the repo pattern is because you can do so much with it. Usually it acts almost as a clear line which defines what should be done directly in the database (meaning a SQL Query) vs. anywhere in code.

The argument against the generic repo is that it cant handle multiple tables via joins. Some say it requires convoluted Linq (which is technically correct). My answer to this is that this is the line I was speaking of earlier. If you find yourself writing long winded Linq queries, or manual queries in C# anywhere the repo pattern is used (specifically the generic version of it), then really this belongs in the DB. So its a clear sign to me that a proper query should be written in SQL, and made into a view which can be properly optimised. Then this view can be brought into the system via EF as a single table and the generic pattern used on it. Hence the generic repo pattern now acts as a clear separation of concerns and serves to highlight where things should be.