ASP.NET MVC using Repository pattern - code review

Question

I am just trying to use ASP.NET MVC using repository pattern. Could somebody see If I am 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

You code shows not only your definition of your repository but also it's usage.

What worries me is that i don't see how the DbContext (AddressBookDB in this case) is disposed off.

I think i'd rather pass in the AddressBookDB object as a paramter to the repository constructor for usage so you can ensure it is diposed off properly by creating it in a using block.

Answer 2

Additional layer depending on app complexity

Your code seems clean and fine but it all boils down to complexity of your application. If business processes are complex I'd suggest to put an additional layer between your controller and repository logic.

So controller would be calling into service, and service would be calling into repositories (even many and several times if it needs to depending on the business process).

Prevent code duplication

Since you've provided such generic repository interface I'd suggest you actually create a generic one to prevent code duplication:

public interface IRepository<TEntity>
{
    void Create(TEntity entity);
    void Delete(int entityId);
    void Save(TEntity entity);
    TEntity Retrieve(int entityId);
    IEnumerable<TEntity> Select();
}

Your repositories would then implement this interface:

public class ContactRepository: IRepository<Contact>
{
    ...
}

And if some repository needs a more powerful interface you can write an additional repository interface, that will likely inherit from this generic IRepository<TEntity> interface.

This would prevent you from having numerous repository interfaces with more or less identical definition.