6
\$\begingroup\$

I have seperated my project into two projects. One is the web forms project and the other is the Model project with contains a generic repository implementation, a service class for each of my entities and a UnitOfWork class. All the code below apart from the last section is in my Model class library project.

Generic repository interface

public interface IRepository<TEntity> : IDisposable where TEntity : class
{
    IQueryable<TEntity> GetAll();
    IQueryable<TEntity> GetWhere(Expression<Func<TEntity,bool>> predicate);
    TEntity GetSingle(Expression<Func<TEntity, bool>> predicate);
    TEntity GetSingleOrDefault(Expression<Func<TEntity, bool>> predicate);

    void Add(TEntity entity);
    void Delete(TEntity entity);
}

I start of with this Interface which my services will interact with.

A few points about this class:

  • As i am having a service for each of my entity object, I see no reason why I shouldn't use a generic repository as creating a repository for each entity object as-well, would bloat the amount of classes i have and make it harder to manage, when i can just pass a predicate from my service into the Get methods to retrieve the data i need. So instead of creating multiple methods such as "GetById", "GetByName" in a repository specific to each entity i use a generic one and include those methods in the service class. However i have heard many people say the generic repository is an anti-pattern so i'm not sure if this is the right way.
  • I do not include a save changes methods in the repositories as i want to make any interactions with these only come from the UnitOfWork class (which will expose the service classes as properties). So this should force validation to be done via the service classes as these are the only objects which will interact with repository. In every example i see the repository has a save changes method so i'n not sure if this is a reasonable approach.

Generic repository

Below is the implementation of the IRepository for my web app, I think it has a pretty standard structure. I swap this out for another implementation when testing the service classes. I don't think there is much to say about it other than what i mentioned above.

internal class RepositoryBase<TEntity> : IRepository<TEntity> where TEntity : class
{
    private bool isDisposed;

    private DbSet<TEntity> dbSet;
    public DbSet<TEntity> DbSet
    {
        get { return dbSet; }
    }

    private RequestboxEntities requestboxContext;

    public RepositoryBase(RequestboxEntities requestboxContext)
    {
        this.requestboxContext = requestboxContext;
        this.dbSet = requestboxContext.Set<TEntity>();
    }

    public IQueryable<TEntity> GetAll()
    {
        return dbSet;
    }

    public IQueryable<TEntity> GetWhere(System.Linq.Expressions.Expression<Func<TEntity, bool>> predicate)
    {
       return dbSet.Where(predicate);
    }

    public TEntity GetSingle(System.Linq.Expressions.Expression<Func<TEntity, bool>> predicate)
    {
        return dbSet.Single(predicate);
    }

    public TEntity GetSingleOrDefault(System.Linq.Expressions.Expression<Func<TEntity, bool>> predicate)
    {
        return dbSet.SingleOrDefault(predicate);
    }

    public void Add(TEntity entity)
    {
        dbSet.Add(entity);
    }

    public void Delete(TEntity entity)
    {
        dbSet.Remove(entity);
    }

    public virtual void Dispose(bool isManuallyDisposing)
    {
        if (!isDisposed)
        {
            if (isManuallyDisposing)
                requestboxContext.Dispose();
        }

        isDisposed = true;
    }

    public void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }

    ~RepositoryBase()
    {
        Dispose(false);
    }

} 

Service classes

These are the only classes that will interact with the generic repository and will be used in the UnitOfWork class. Below is part of one of my service classes, i have left out some code to improve readability, all other services are very similar to this one.

public class GroupService
{
    IUnitOfWork unitOfWork;
    IRepository<Group> groupRepository;

    public GroupService(IRepository<Group> groupRepository, IUnitOfWork unitOfWork)
    {
        this.groupRepository = groupRepository;
        this.unitOfWork = unitOfWork;
    }

    public Group GetById(int id)
    {
        return groupRepository.GetSingle(g => g.Id == id); 
    }

    public List<Group> GetProductGroupsUserBelongsTo(int instanceId, Guid userId)
    {
        IQuerable<UserGroup> userGroups = unitOfWork.UserGroupService.GetByUserId(userId);
        IQuerable<Group> groupsInInstance = groupRepository.GetWhere(g => g.InstanceId == instanceId);

        return groupsInInstance.Join(userGroups, g => g.Id, u => u.GroupId, (g, u) => g).ToList();
    }

    public void Add(Group group)
    {
       if(!ExistsInInstance(group.Name,group.InstanceId))
           groupRepository.Add(group);
       else 
           throw new Exception("A group with the same name already exists for the instance");          
    }

    public bool ExistsInInstance(string name, int instanceId)
    {
        return GetAll().SingleOrDefault(g => g.Name == name && g.InstanceId == instanceId) != null;
    }
}

Some points and concerns about these classes

  • I have made these classes require a IUnitOfWork class in the constructor. This is for functions which require interacting with a repository other than it's own. In the "GetProductGroupsUserBelongsTo" method i need to read from my UserService class aswell so instead of creating a new UserService object i use the existing one in the UnitOfWork object passed through to the service. I did this as it ensures I always use the same entity context (set in the UnitOfWork class) when using different services/repositories. Doing it this way also makes sure i don't use the context directly in my service classes, which makes them easier to test.
  • In my Get Methods i generally use GetSingle() so an error is thrown if an object doesnt exist for the Id. However i am not sure if i should be using GetSingleOrDefault() and then handling the null on the client that consumes the service or if i should expect the client to use the "ExistsInInstance" method to check if it exists before calling the Get Methods.

UnitOfWork interface

I have removed some of the services for readability purposes, but there are many more services defined as properties just like the "GroupService".

public interface IUnitOfWork : IDisposable
{
    GroupService GroupService { get; }
    UserService UserService { get; }

    void Commit();
}

UnitOfWork Class

 public class UnitOfWork : IUnitOfWork
{
    private bool disposed = false;
    private RequestboxEntities requestboxContext = new RequestboxEntities();

    private GroupService groupService;
    private UserService userService;

    public GroupService GroupService
    {
        get
        {
            if (groupService == null)
                groupService = new GroupService(new RepositoryBase<Group>(requestboxContext));

            return groupService;
        }

    }

    public UserService UserService
    {
        get
        {
            if (userService == null)
                userService = new UserService(new RepositoryBase<User>(requestboxContext));

            return userService;
        }

    }

    public UnitOfWork()
    {
        requestboxContext = new RequestboxEntities();
    }

    public void Commit()
    {
        requestboxContext.SaveChanges();
    }

    public void Refresh()
    {
        requestboxContext.Dispose();
        requestboxContext = new RequestboxEntities();
    }

    protected virtual void Dispose(bool isManuallyDisposing)
    {
        if (!this.disposed)
        {
            if (isManuallyDisposing)
                 requestboxContext.Dispose();
        }

        this.disposed = true;
    }

    public void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }

    ~UnitOfWork()
    {
        Dispose(false);
    }
}

A few points about this class:

  • Each service is exposed as a public property. This is so when working with it you can easily call the methods like this "unitOfWork.GroupService.GetById(id)"

  • This class shares the same entity context between each service, so to ensure when working with the same unitOfWork object everything is kept in sync. This is the only class that can save the context via the "Commit" method.

Working with the Model

So that's the structure of my Model project here is an example if how i consume it in a web page. I know this might fit better in an MVC project rather than webforms but that is not a realistic option at the moment and my main concerns are with the structure and use of the Model project not the web app.

 public class BasePage : System.Web.UI.Page
{
   UnitOfWork uow = new UnitOfWork();

   protected override void OnUnload(EventArgs e)
    {
        uow.Dispose();
        base.OnUnload(e);
    }
}

Each page will inherit this class so a unique UnitOfWork will be made for each request and will always be disposed on the "Unload" event. Then to use it in a page i can do something like this.

 public class ManageGroupsPage : BasePage
{
   protected void Page_Load(object sender, EventArgs e)
    {
        int groupId = (int)dropdown.SelectedItem.Value;
        Group group = uow.GroupService.GetById(groupId);

        NameTextBox.text = group.Name;
    }
}

I can then easily and quickly work with the UnitOfWOrk object by interacting with it's public properties (The Service classes).

Conclusion and Concerns

My main concerns are with the structure of the Model project and how the layers interact with each other. Whether it is a good idea to use the generic repository, if the service classes should be passed through a IUnitOfWork object as this prevents them from working without this class so it's not as flexible. And if it is a good idea to have evrything rely on the UnitOfWork class.

\$\endgroup\$
  • 1
    \$\begingroup\$ Welcome. Nice complete question. Hope you find the answers your looking for. \$\endgroup\$ – dreza Apr 22 '14 at 10:54
5
\$\begingroup\$

I've used the Generic repository solution myself in the past. At first I found it really great but after a while as the project got a bit bigger it started to feel a bit restrictive and didn't seem to offer much benefits for the amount of code written.

The more I read the more it appeard Entity framework was already a Repository / UnitofWork. Why did I need another layer? Well I'm sure there are plenty of reasons why but in my projects I couldn't see it.

Instead I found myself starting to use the entity-framework directly within a service layer but abstracting the EF away using interfaces. This allowed me to remove the Repository layer whilst maintaining the level of abstraction I was after between the UI layer and any business or data layer (as well as continuing to easily mock for unit tests).

Using your solution an alternative approach I might consider would be something like:

public interface IUnitOfWork
{
   public IDbSet<Group> Groups { get; set; }
   // other entity sets here

   System.Data.Entity.Database Database { get; }
   DbEntityEntry<TEntity> Entry<TEntity>(TEntity entity) where TEntity : class;
   int SaveChanges();
}

My Unit of work was essentially my Entity framework data context

public class MyDbContext : DbContext, IUnitOfWork
{
   public DbSet<Group> Groups { get; set; }

   // etc
}

I would now only use a service layer directly interacting with the IUnitOfWork. However I would also consider abstracting each service behind an interface itself.

public interface IGroupService : ICrudService<Group> // Maybe, maybe not depending
{
    Group GetById(int id);
}

If there were a group of interfaces that shared a common theme i.e. CRUD I might consider making those seperate and implementing them where necessary. Or even a base service class to contain common methods.

public interface ICrudService<TEntity>
{
   TEntity GetById(int id);
   void Update(TEntity entity);
   void Add(TEntity entity);
   void Delete(TEntity entity);
}

public class GroupService : IGroupService
{
   private readonly IUnitOfWork _unitOfWork;

   public GroupService(IUnitOfWork unitOfWork)
   {
      _unitOfWork = unitOfWork;
   } 

   public Group GetById(int id)
   {
      return _unitOfWork.Find(id);
   }
}

If my service required other services I would simply pass those into the constructor. If I found myself passing too many then I would start knowing I had a bit of SRP mis-use and consider creating another service or re-arranging my existing code base.

If you wanted a GOD service class you could still achieve this as you did with your UnitOfWork example by doing something like:

public interface IServiceContext
{
   IGroupService GroupService { get; private set; }
   // And all your other services
}

Hopefully you get the drift....

\$\endgroup\$
  • \$\begingroup\$ Thanks for the feedback. I think i understand, so your UnitOfWork class would act like the repository as-well including methods like "Find". (For my example i may add "GetSingle" and "GetWhere" as seen in my original post). Should these methods also be in the IUnitOfWork interface? And then when you come to consume the services you would create an instance of the service and pass through the UOW, instead of interacting with the UnitOfWork directly (via it's properties) as i did. Am I understanding this correctly? \$\endgroup\$ – user2945722 Apr 22 '14 at 11:12
  • \$\begingroup\$ Ah I see, so you would manually go into the DbContext class and make it implement your IUnitOfWork (I am using db first so this is generated for me, but i could still edit it every time i regenerate the database) So the interface would not need the methods like "GetWhere" defined as my original comment suggested. \$\endgroup\$ – user2945722 Apr 22 '14 at 11:26
  • 1
    \$\begingroup\$ @user2945722 rather than manually edit the auto generated class you could create a partial DbContext and make that one implement your interface \$\endgroup\$ – dreza Apr 22 '14 at 21:55
  • \$\begingroup\$ @user2945722 I guess I would have my service layer do most of the logic. So needing a generic GetWhere off that might not be needed as instead it would expose methods like GetActiveGroup and GetRelatedGroupsTo i.e. buiness rule type methods \$\endgroup\$ – dreza Apr 22 '14 at 21:56

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.