13
\$\begingroup\$

I have a question about the Repository pattern:

public abstract class RepositoryBase<T> : IDisposable, IRepository<T> where T : class, IEntity
    {
        /// <summary>
        /// For iterations using LINQ extension methods on the IQueryable and IEnumerable interfaces
        /// it is required that the context which was used to make the query was not disposed before the iteration
        /// otherwise it will throw an exception
        /// </summary>
        private Context _context;

        protected Context Context
        {
            get { return _context ?? (_context = new Context()); }
        }

        public virtual void AddOrUpdate(T entity)
        {
            var dbSet = Context.Set<T>();

            if (dbSet.Contains(entity))
                entity.ChangeDateTime = DateTime.Now;
            else
                entity.AddeDateTime = DateTime.Now;

            Context.Set<T>().AddOrUpdate(entity);
            Context.SaveChanges();
        }

        public virtual void Delete(T entity)
        {
            Context.Set<T>().Remove(entity);
            Context.SaveChanges();
        }

        public void Drop()
        {
            var dbSet = Context.Set<T>();

            foreach (var entity in dbSet)
                dbSet.Remove(entity);

            Context.SaveChanges();
        }

        public virtual IQueryable<T> Find(Expression<Func<T, bool>> predicate)
        {
            return Context.Set<T>().Where(predicate);
        }

        public IEnumerable<T> GetAll()
        {
            return Context.Set<T>();
        }

        public void Dispose()
        {
            if (_context != null)
            {
                _context.Dispose();
                _context = null;
            }
        }
    }

Why I am not embracing each method implementation with a using and disposing the Context as soon as the query is done?

Some of my methods, as Find and GetAll return collections. I would want to be able to iterate through them using LINQ, only this is not possible if the Context is disposed, as it would be in the original Repository model.

Is this the best approach to go around this issue?

\$\endgroup\$
2
  • \$\begingroup\$ Is IEntity just a marker interface? \$\endgroup\$ Jan 25, 2014 at 20:00
  • \$\begingroup\$ Yes, it is just a marker interface. \$\endgroup\$ Jan 25, 2014 at 20:16

5 Answers 5

4
\$\begingroup\$

My question is about why I am not embracing each method implementation with a using and disposing the Context as soon as the query is done. Some of my methods, as Find and GetAll return collections. I would want to be able to iterate through them using LINQ, only this is not possible if the Context is disposed, as it would be in the original Repository model.

A Linq enumerable isn't enumerated when it's created: it's enumerated when you try to work with it.

If we want to dispose the context within each each method, I think you can do that by realizing the data, i.e. by reading it all into a concrete object such List or EnumerableQuery, before you dispose the context ... something like this (untested code ahead):

public IEnumerable<T> GetAll()
{
    using (Context context = new Context())
    {
        IEnumerable<T> enumerable = context.Set<T>();
        // enumerate into a List before disposing the context
        List<T> list = new List<T>(enumerable);
        return list;
    }
}

public virtual IQueryable<T> Find(Expression<Func<T, bool>> predicate)
{
    using (Context context = new Context())
    {
        IEnumerable<T> enumerable = context.Set<T>().Where(predicate);
        // enumerate into a EnumerableQuery before disposing the context
        // see https://stackoverflow.com/a/6765404/49942 for further details
        EnumerableQuery<T> queryable = new EnumerableQuery<T>(enumerable);
        return queryable;
    }
}

Beware that this is expensive if the data set is huge, if you don't actually want all the data you queried.

\$\endgroup\$
3
\$\begingroup\$

Reusing the same context could have some benefits if you need entities to be cached but you could also end up with contexts with a large memory sizes.

Depending on the requirements, I have usually created a new context on every call and in the case of database contexts, played it safe by forcing a ToList() within the function, before returning the entities.

This ensures that the database connection is only open for just enough time to retrieve the rows and the context is destroyed without holding on to the entities retrieved.

The approach to take here, really depends on your requirements.

\$\endgroup\$
2
  • \$\begingroup\$ But are you able to use LINQ if you use ToList() on the collection? Even after the context was disposed? \$\endgroup\$ Jan 25, 2014 at 22:22
  • \$\begingroup\$ The ToList() needs to be called before the context is disposed. I noticed the 'Find' returns an IQueryable instead of an IEnumerable. Are you expecting the caller to further augment the query? If not, the 'Find' could return an IEnumerable or IList, after calling ToList(). \$\endgroup\$
    – hocho
    Jan 25, 2014 at 22:41
2
\$\begingroup\$

Firstly I would consider injecting the DbContext into the repository. Even though your RepositoryBase class is abstract I would have a protected constructor

protected RepositoryBase(Context dbContext) {
  _context = dbContext;
}

I would then look at injecting the context into each repository and letting object that ends up creating the context be responsible for it's disposing. Perhaps you could take a look at the UnitOfWork pattern as that goes well with the repository pattern (There are plenty of articles online).

In light of this I then probably would let another class/object be responsible for when the changes are persisted to the database. Perhaps look at it like a UI that lets you add items to a list as much as you want. Then there is a save button that will actually do the persist to the db. This save button is separate to the adding itself and means that you can bulk add items and only save when you are ready.

One other point of doing it this way is that you never get to share Context. That means that each repositories set of changes will not be visible in another set (until the changes are persisted). This maybe what you are after but if not, it's something to consider.

\$\endgroup\$
2
\$\begingroup\$

The container calls Dispose for the IDisposable types it creates. Services resolved from the container should never be disposed by the developer. If a type or factory is registered as a singleton, the container disposes the singleton automatically.

Source: https://docs.microsoft.com/en-us/aspnet/core/fundamentals/dependency-injection?view=aspnetcore-2.1#disposal-of-services-1

UPDATE:

So, simply by adding IDisposable in your IRepositoryWrapper interface or RepositoryWrapper class and dispose your DatabaseContext as flow:

public class RepositoryWrapper: IDisposable 
{
    private MyDbContext _repositoryContext;
    
    //Ctor...
    //.....

    public void Dispose()
    {
        this._repositoryContext.Dispose();
    }
}

The Dispose is automatically called by the container.

\$\endgroup\$
1
\$\begingroup\$

The answer is the Unit Of Work pattern. You are right that using is modern only it can not be used if you have a Unit Of Work in your repository that has lets say 3 write actions in it that are part of ONE transaction, but can be split in 3 writing methods. For those 3 write actions you will need the same context from the same repository. Most of those times there will be a Commit method in that repository to "Commit" that atomary transaction. You can then also Implement a "Rollback" method if one of those 3 write actions will fail.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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