1
\$\begingroup\$

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?

\$\endgroup\$
  • \$\begingroup\$ Why are you returning anything from Add and Update? \$\endgroup\$ – abuzittin gillifirca Mar 27 '13 at 7:12
  • \$\begingroup\$ I'm not quite sure how I feel about abstracting away something that is already quite heavy abstraction layer. What if you want to use Include for eager loading? Or GroupJoin for LEFT OUTER JOIN between unrelated entities? What if you want to select only 2 properties out of 20 in the entity? When just using EF, you could project the to anonymous type and generated SQL would be quite bearable, but you can't do it here... \$\endgroup\$ – Patryk Ćwiek Dec 16 '13 at 12:05
  • \$\begingroup\$ Ask yourself: what is the value of this abstraction? Try to be as concrete as possible. You'll notice: there's not a lot there. \$\endgroup\$ – usr Dec 18 '13 at 16:39
3
\$\begingroup\$

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.

\$\endgroup\$

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.