Is this correct usage of Repository + Unit of Work + Service Pattern

Question

I have project where I need to use C# and Entity Framework.

After my question here, I have decided to use Unit of Work and Repository patterns.

This is the first time when I'm using this. Can anyone tell me if I'm doing this right or I have to redesign something?

BTW: why do I have to set student.Group = null in Service.StudentService.Update()? Without this I'm getting exception. I don't understand when should I use student.Group, and when student.GroupId.

Entities:

namespace Model {
 public abstract class BaseEntity {
 [Timestamp]
 public Byte[] TimeStamp{get; set;}
 }
}
namespace Model {
 public class Group : BaseEntity {
 [Key]
 public int GroupId {get; set;}
 public string Name {get; set;}
 public virtual ICollection<Student> Students {get; set;}
 }
}
namespace Model {
 public class Student : BaseEntity {
 [Key]
 public int StudentId {get; set;}
 public string FirstName {get; set;}
 public string LastName {get; set;}
 public string IndexNo {get; set;}
 public int GroupId {get; set;}
 [ForeignKey("GroupId")]
 public virtual Group Group {get; set;}
 public virtual ICollection<Registration> Registrations {get; set;}
 }
}

Context:

namespace Model {
 public class StorageContext : DbContext {
 public DbSet<Group> Groups {get; set;}
 public DbSet<Student> Students {get; set;}
 ...
 }
}

Repository:

namespace Repository {
 public class GenericRepository<TEntity> where TEntity : BaseEntity {
 internal StorageContext context;
 internal DbSet<TEntity> dbSet;
 public GenericRepository(StorageContext context) {
 this.context = context;
 this.dbSet = context.Set<TEntity>();
 }
 public virtual IEnumerable<TEntity> Get(
 Expression<Func<TEntity, bool>> filter = null,
 Func<IQueryable<TEntity>, IOrderedQueryable<TEntity>> orderBy = null,
 string includeProperties = "") {
 IQueryable<TEntity> query = dbSet;
 if (filter != null) {
 query = query.Where(filter);
 }
 foreach (var includeProperty in
 includeProperties.Split(new char[] { ',' }, StringSplitOptions.RemoveEmptyEntries)) {
 query = query.Include(includeProperty);
 }
 if (orderBy != null) {
 return orderBy(query).ToList();
 } else {
 return query.ToList();
 }
 }
 public virtual TEntity GetById(object id) {
 return dbSet.Find(id);
 }
 public virtual void Insert(TEntity entity) {
 dbSet.Add(entity);
 }
 public virtual void Delete(object id) {
 TEntity entityToDelete = dbSet.Find(id);
 Delete(entityToDelete);
 }
 public virtual void Delete(TEntity entity) {
 if (context.Entry(entity).State == EntityState.Detached) {
 dbSet.Attach(entity);
 }
 dbSet.Remove(entity);
 }
 public virtual void Update (TEntity entity) {
 dbSet.Attach(entity);
 context.Entry(entity).State = EntityState.Modified;
 }
 }
}

UnitOfWorks:

namespace Repository {
 public class GenericUnitOfWork<TEntity> : IDisposable where TEntity : BaseEntity{
 public readonly StorageContext Context;
 private bool _disposed;
 private GenericRepository<TEntity> repository;
 public GenericRepository<TEntity> GetRepository() {
 return repository;
 }
 public GenericUnitOfWork(StorageContext context) {
 this.Context = context;
 repository = new GenericRepository<TEntity>(Context);
 }
 public IEnumerable<TEntity> Get() {
 return repository.Get(null, null, "");
 }
 public virtual void Insert(TEntity entity) {
 repository.Insert(entity);
 Save();
 }
 public virtual void Update(TEntity entity) {
 repository.Update(entity);
 Save();
 }
 public void Delete(object id) {
 repository.Delete(id);
 Save();
 }
 public void Save() {
 Context.SaveChanges();
 }
 public void Dispose() {
 Dispose(true);
 GC.SuppressFinalize(this);
 }
 protected virtual void Dispose(bool disposing) {
 if (!this._disposed) {
 if (disposing) {
 Context.Dispose();
 }
 this._disposed = true;
 }
 }
 }
}
namespace Repository {
 public class GroupUnitOfWork : GenericUnitOfWork<Group>{
 private GenericRepository<Group> repository;
 public GroupUnitOfWork(StorageContext context) : base(context){
 repository = new GenericRepository<Group>(Context);
 }
 public IEnumerable<Group> GetIncludeStudents() {
 return repository.Get(null,null,"Students");
 }
 }
}
namespace Repository {
 public class StudentUnitOfWork : GenericUnitOfWork<Student> {
 private GenericRepository<Student> repository;
 public StudentUnitOfWork(StorageContext context) : base(context){
 repository = new GenericRepository<Student>(Context);
 }
 public IEnumerable<Student> GetByGroupId(object groupId) {
 return repository.Get(s => s.GroupId == (int)groupId, null, "Group");
 }
 public override void Insert(Student student) {
 repository.Insert(student);
 Save();
 }
 public override void Update(Student student) {
 repository.Update(student);
 Save();
 }
 }
}

Services:

namespace Service {
 public class GenericService<TEntity> where TEntity : BaseEntity{
 protected StorageContext context;
 public IEnumerable<TEntity> Get() {
 using(var context = new StorageContext()) {
 return new GenericUnitOfWork<TEntity>(context).Get();
 }
 }
 public virtual void Insert(TEntity entity) {
 using(var context = new StorageContext()) {
 new GenericUnitOfWork<TEntity>(context).Insert(entity);
 }
 }
 public virtual void Update(TEntity entity) {
 using(var context = new StorageContext()) {
 new GenericUnitOfWork<TEntity>(context).Update(entity);
 }
 }
 public void Delete(object id) {
 using(var context = new StorageContext()) {
 new GenericUnitOfWork<TEntity>(context).Delete(id);
 }
 }
 }
}
namespace Service {
 public class GroupService : GenericService<Group>{
 public IEnumerable<Group> GetIncludeStudents() {
 using(var context = new StorageContext()) {
 return new GroupUnitOfWork(context).GetIncludeStudents();
 }
 }
 }
}
namespace Service {
 public class StudentService : GenericService<Student> {
 public IEnumerable<Student> GetByGroupId(object id) {
 using(var context = new StorageContext()) {
 return new StudentUnitOfWork(context).GetByGroupId(id);
 }
 }
 public override void Update(Student student) {
 using(var context = new StorageContext()) {
 //without set student.Group to null I'm getting exception.
 student.Group = null;
 new StudentUnitOfWork(context).Update(student);
 }
 }
 }
}

• Mathieu Guindon: Group entity vs GroupId: choose one, stick to it. You can specify either, or both. I think you're getting an exception because your navigation property isn't virtual.
• WojciechKo: True, I've overlooked this. But still, I'm getting: A referential integrity constraint violation occurred: The property values that define the referential constraints are not consistent between principal and dependent objects in the relationship. exception
• stink: Can you post your controller... that's really where it matters the most.

Answer 1

That looks pretty good but it depends on what your controller looks like.

There is a valid argument that the Entity Framework implements the Repository Pattern for you and all you need to implement is the service layer.

So you could put the service layer directly on top of the dbSet. Since I started using just the service layer without the repo pattern directly (so the service layer accesses DbSet) I have not had any problems.

---

DbSet == Repository
DbContext == Unit of Work

---

Here is what a controller with the repo looks like. If yours is similar I would say that your code is correct.

Cheeseburger Controller

namespace WickedAwesomeWebApp
{ 
 public class CheeseburgerController : Controller
 {
 public UnitOfWork unitOfWork = new UnitOfWork();
 public CheeseburgerRepository cheeseburgerRepository = new CheeseburgerRepository(new MyDbContext());
 public ViewResult Index()
 {
 var viewModel = new CheeseburgerViewModel();
 viewModel.Cheeseburger = unitOfWork.CheeseburgerRepository.Get();
 return View(viewModel);
 }
 // GET: /Cheeseburger/Details/5
 public ViewResult Details(int id)
 {
 return View(unitOfWork.CheeseburgerRepository.GetById(id));
 }
 // GET: /Cheeseburger/Create
 public ActionResult Create()
 {
 return View();
 } 
 // POST: /Cheeseburger/Create
 [HttpPost]
 public ActionResult Create ([Bind(Include = {CheeseburgerId,/*Include Object Properties to Bind*/{)]Cheeseburger cheeseburger)
 {
 if (ModelState.IsValid) {
 unitOfWork.CheeseburgerRepository.Insert(cheeseburger);
 unitOfWork.CheeseburgerRepository.Update(cheeseburger);
 return RedirectToAction({Index{);
 } else {
 return View();
 }
 }
 // GET: /Cheeseburger/Edit/5
 public ActionResult Edit(int id)
 {
 return View(unitOfWork.CheeseburgerRepository.GetById(id));
 }
 // POST: /Cheeseburger/Edit/5
 [HttpPost]
 public ActionResult Edit(Cheeseburger cheeseburger)
 {
 if (ModelState.IsValid) {
 unitOfWork.CheeseburgerRepository.Update(cheeseburger);
 unitOfWork.Save();
 return RedirectToAction({Index{);
 } else {
 return View();
 }
 }
 // GET: /Cheeseburger/Delete/5
 public ActionResult Delete(int id)
 {
 return View(unitOfWork.CheeseburgerRepository.GetById(id));
 }
 // POST: /Cheeseburger/Delete/5
 [HttpPost, ActionName({Delete{)]
 public ActionResult DeleteConfirmed(int id)
 {
 unitOfWork.MealRepository.Delete(id);
 unitOfWork.Save();
 return RedirectToAction({Index{);
 }
 protected override void Dispose(bool disposing)
 {
 if (disposing) {
 unitOfWork.Dispose();
 }
 base.Dispose(disposing);
 }
 }
}

ICheeseburgerRepository

namespace WickedAwesomeWebApp
{
 public interface ICheeseburgerRepository : IDisposable
 {
 IQueryable Cheeseburgers { get; }
 IEnumerable GetCheeseburgers();
 Cheeseburger GetCheeseburgerById(int? id);
 void InsertCheeseburger(Cheeseburger id);
 void DeleteCheeseburger(int cheeseburgerId);
 void UpdateCheeseburger(Cheeseburger cheeseburger);
 void Save();
 new void Dispose();
 }

CheeseburgerRepository With CRUD

 public class CheeseburgerRepository : ICheeseburgerRepository
 {
 private SkimosContext context = new SkimosContext();
 public CheeseburgerRepository(SkimosContext context)
 {
 this.context = context;
 }
 public IQueryable Cheeseburgers
 {
 get { return context.Cheeseburgers; }
 }
 public IEnumerable GetCheeseburgers()
 {
 return context.Cheeseburgers.ToList();
 }
 public Cheeseburger GetCheeseburgerById(int? id)
 {
 return context.Cheeseburgers.Find(id);
 }
 public void InsertCheeseburger(Cheeseburger cheeseburger)
 {
 context.Cheeseburgers.Add(cheeseburger);
 }
 public void DeleteCheeseburger(int cheeseburgerId)
 {
 Cheeseburger cheeseburger = context.Cheeseburgers.Find(cheeseburgerId);
 context.Cheeseburgers.Remove(cheeseburger);
 }
 public void UpdateCheeseburger(Cheeseburger cheeseburger)
 {
 context.Entry(cheeseburger).State = System.Data.Entity.EntityState.Modified;
 }
 public void Save()
 {
 context.SaveChanges();
 }
 private bool disposed;
 protected virtual void Dispose(bool disposing)
 {
 if (!disposed)
 {
 if (disposing)
 {
 context.Dispose();
 }
 }
 disposed = true;
 }
 public void Dispose()
 {
 if (context != null) context.Dispose();
 }
 }
}

• user441521: Isn't the UnitOfWork supposed to contain the repositories and the DB context? Isn't that sort of the point of the UnitOfWork?
• stink: No your coupling the unit of work and repositories together. They are different patterns commonly used together.
• Rick the Scapegoat: Not coupling implementations - but unit of work should have repository interfaces exposed.