Abstract repository for entity framwork

Question

I write abstract repository

using System;
using System.Collections.Generic;
using System.Data;
using System.Data.Entity;
using System.Linq;
using System.Linq.Expressions;

namespace Framework.DataLayer
{
    public class Context<T> : IContext<T> where T : class
    {
        private readonly DbContext _dbContext;

        public Context(DbContext dbContext)
        {
            _dbContext = dbContext;
        }

        public bool TryGet(Expression<Func<T, bool>> predicate, out T entity)
        {
            entity = List(predicate).SingleOrDefault();
            return entity != null;
        }

        public T Get(Expression<Func<T, bool>> predicate)
        {
            return List(predicate).Single();
        }

        public List<T> List(Expression<Func<T, bool>> predicate = null)
        {
            IQueryable<T> result = _dbContext.Set<T>().AsQueryable();
            if (predicate != null)
                result = result.Where(predicate);
            return result.ToList();

        }

        public T Add(T t)
        {
            _dbContext.Entry(t).State = EntityState.Added;
            return t;

        }

        public void Delete(Expression<Func<T, bool>> predicate)
        {
            List(predicate).ToList().ForEach(p => { _dbContext.Entry(p).State = EntityState.Deleted; });
        }

        public void Delete(T entity)
        {
            _dbContext.Entry(entity).State = EntityState.Deleted;
        }

        public T Update(T t)
        {
            _dbContext.Entry(t).State = EntityState.Modified;
            return t;

        }

    }
}

Is this correct?

Answer 1

For one, it is not an abstract repository, but a generic repository. Never mind. As mentioned in a comment, generic repositories are hardly ever helpful and I would never use them. But apart from that, there's a couple of things I can say about this code:

• TryGet should be absolutely safe (that's what the "Try" prefix conveys), so use FirstOrDefault() and only return an entity if exactly one is fetched.

• I would not return a List, but an IEnumerable, so you retain deferred execution.

• If the repository is only internally used, e.g. to supply data for services, I would return IQueryable, so you can compose queries from different repositories, do projections, includes and everything you can do with IQueryable and not with IEnumerable.

• As said in another comment, it is not necessary to return anything from Add and Update, because you always modify a reference type.

• I would prefer method names like MarkForDelete, because that's what actually happens.