Generic repository pattern

Question

Please have a through look my custom implementation of generic repository pattern and suggest anything I may be lacking.

1. I am using a generic interface with 6 methods
2. I'll be using a repository class for each entity type (e.g UserRepository, ProductRepository, StoreRepository) which are not only going to implement the generic interface but also can add additional methods as per the need with respect to each repository class. for example:
   • UserRepository can add a new method named as CountUsers() which gets called from controller.
   • ProductRepository can add a new method named as GetExpiredProducts() which gets called from controller.
3. I am declaring IGenericRepository<User> in UserController, similarly in other controllers and using repositoryPattern.

Generic Repository Interface

public interface IGenericRepository<T> : IDisposable where T : class
{
    IEnumerable<T> SelectAll();
    T SelectByID(object id);
    void Insert(T obj);
    void Update(T obj);
    void Delete(object id);
    void Save();
}

Implementation of Generic Repository Interface

public class UserRepository : IGenericRepository<User>
{
    private ModelDBEntities dataContext = null;
    public UserRepository()
    {
        this.dataContext = new ModelDBEntities();
    }
    public UserRepository(ModelDBEntities dataContext)
    {
        this.dataContext = dataContext;
    }

    public IEnumerable<User> SelectAll()
    {
        return dataContext.Users;
    }
    public User SelectByID(object argId)
    {
        long userId = long.Parse(argId.ToString());
        return dataContext.Users.Single(u => u.UserId == userId);
    }
    public void Insert(User obj)
    {
       dataContext.Users.AddObject(obj);
    }
    public void Update(User obj)
    {
        if (obj.EntityState == EntityState.Detached)
            dataContext.Users.Attach(obj);
        dataContext.ObjectStateManager.ChangeObjectState(obj, System.Data.EntityState.Modified);            
    }
    public void Delete(object argUserId)
    {
        long userId = long.Parse(argUserId.ToString());
        dataContext.Users.Single(u => u.UserId == userId).Status = "inactive";
    }
    public void Save()
    {
        dataContext.SaveChanges();
    }

    private bool disposed = false;
    protected virtual void Dispose(bool disposing)
    {
        if (!this.disposed)
        {
            if (disposing)
            {
                dataContext.Dispose();
            }
        }
        this.disposed = true;
    }
    public void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }
}

My User Controller

public class UserController : Controller
{
    private IGenericRepository<User> _repositoryUser = null;     
    public UserController()
    {
        _repositoryUser = new UserRepository();
    }
    public UserController(IGenericRepository<User> repository)
    {
        _repositoryUser = repository;
    }

    [HttpPost]
    public JsonResult SubmitHandler(User argUser)
    {
        if (ModelState.IsValid)
        {
            _repositoryUser.Insert(argUser);
            _repositoryUser.Save();
            return Json("success");
        }
        else
            return Json("failure");
    }
    public ActionResult Delete(object Id)
    {
            if (Id != null && Id.ToString().Trim().Length > 0)
            {
                int param = int.Parse(Id.ToString());
                _repositoryUser.Delete(Id as object);
                _repositoryUser.Save();
                return View("User", _repositoryUser.SelectAll());
            }
            else
                return View("404");
    }
}

Answer 1

I have created generic repositories many times and what I found was that you're going to need to create interfaces for each entity typed repository.

So IUserRepository that implements IGenericRepository<User>, this way you'll be able to

• Inject the repository easier with Inversion Of Control
• Create extra methods for each entity type as and when
• Test your controller easier

Update

I realise this update comes a long time after the question, but time has past and experience has been gained.

You don't necessarily need a new interface for entity type. IoC containers can inject a specific implementation based on the generic parameter of the IGenericRepositoy interface. You will need an interface however for each implementation that has specific methods for that type, such as GetExpiredProducts() as mentioned in the initial question.

You could also remove the generic parameter from the interface and move the where clause to each method:

public interface IGenericRepository : IDisposable
{
    IEnumerable<T> SelectAll() where T : class;
    T SelectByID(object id) where T : class;
    void Insert(T obj) where T : class;
    void Update(T obj) where T : class;
    void Delete(object id);
    void Save();
}

then implement a GenericRepository class:

public class GenericRepository : IGenericRepository
{
    public GenericRepository()
    {
        blah...  
    }
    public GenericRepository(ModelDBEntities dataContext)
    {
        blah...  
    }

    public IEnumerable<T> SelectAll() where T : class
    {
        blah...  
    }
    public T SelectByID(object argId) where T : class
    {
        blah...  
    }
    public void Insert(T obj) where T : class
    {
       blah...  
    }
    public void Update(T obj) where T : class
    {
        blah...            
    }
    public void Delete(object argId)
    {
        blah...  
    }
    public void Save()
    {
        blah...  
    }
}

this would mean that any implementation of a type specific repository could just inherit this saving you some work

public interface IUserRepository : IGenericRepository
{
    int CountUsers();
}

public class UserRepository : GenericRepository, IUserRepository
{
    public UserRepository()
    {
        blah...  
    }
    public UserRepository(ModelDBEntities dataContext)
        : base(dataContext)
    {
        blah...  
    }
    
    public int CountUsers()
    {
        blah...
    }
}

You can then decide on what you want to inject into your controller, if you just want the base CRUD methods then you will only need to pass in the implementation of IGenericRepository and use for several types.

Answer 2

Var

Prefer the var keyword when declaring local variables if the right hand side of the declaration makes the variable's type obvious.

int param = int.Parse(Id.ToString());

Should be:

var param = int.Parse(Id.ToString());

Doing this makes it easier to change variable type later, and makes it more concise and easy to read for others.

Braces

Prefer to use curly braces for if, else, etc statements, even if they are a single line. It lets the code breathe a bit and improves readability. Additionally, add a separate such clauses from the rest of the code body with an empty line.

if (obj.EntityState == EntityState.Detached)
    dataContext.Users.Attach(obj);
dataContext.ObjectStateManager.ChangeObjectState(obj, System.Data.EntityState.Modified);

Becomes:

if (obj.EntityState == EntityState.Detached)
{
    dataContext.Users.Attach(obj);
}

dataContext.ObjectStateManager.ChangeObjectState(obj, System.Data.EntityState.Modified);

Validation

You should validate the parameters of your public methods using guard clauses.

For example:

public void Update(User obj)
{
    if (obj.EntityState == EntityState.Detached)
        dataContext.Users.Attach(obj);
    dataContext.ObjectStateManager.ChangeObjectState(obj, System.Data.EntityState.Modified);            
}

Does not test for a null user before using it. The result would be a NullReferenceException when it would be more useful and descriptive to throw an ArgumentNullException

Answer 3

I presume that each of your entities are completely independent from each other, that none of them refer to each other, nor do you perform compound queries. Also, the entities are updated or created at completely different times. Also, you only ever update or query one entity instance at a time.

Since those are unreasonable restrictions on entity design, you have to rethink what you are doing. If you have Stores that hold Products, then either the Store repository has to know about Products or vice versa.

Similarly, proper repository design should support the use of statement of work pattern. Transaction boundaries, validation and similar design issues also come into play.