2
\$\begingroup\$

I'm using VS 2013/EF 6/WEB API 2.

This application of mine which is working on will eventually grow to be big project and I'm in the very beginning stage and need your expertise to make it simple as possible without over-complication.

My goal is to make it KISS solution and I'm trying my best. After spending a lot of time thinking and researching I have come up with this design.

PS: I'm very open for suggestions and willing to change my design please don't be harsh with me.

//DAL project

public interface IRepository<T>
{
    IEnumerable<T> GetAll();
}

//Impl

public class HostRepository : IRepository<Host>
    {
        public IEnumerable<Host> GetAll()
        {
            using (SRDbContext context = new SRDbContext())
            {
                var hosts = from host in context.Hosts
                            select new Host()
                            {
                                FirstName = host.FirstName,
                                LastName = host.LastName,
                                Email = host.Email
                            };                
                return hosts.AsEnumerable().ToList();
            }
            return null;
        }
    }

//UOW

public class UnitOfWork : IDisposable
    {
        private SRDbContext context = new SRDbContext();
        private GenericRepository<Host> hostRepository;

        public GenericRepository<Host> HostRepository
        {
            get
            {
                if (this.hostRepository == null)
                {
                    this.hostRepository = new GenericRepository<Host>(context);
                }
                return hostRepository;
            }
        }
     }

//GenericRepository

public class GenericRepository<T> where T : class
    {
        internal DbContext context;
        internal IDbSet<T> IDbSet;

        public GenericRepository(DbContext context)
        {
            this.context = context;
            this.IDbSet = context.Set<T>();
        }

        public virtual T GetByID(object id)
        {
            return IDbSet.Find(id);
        }

        public IEnumerable<T> GetAll()
        {
            IQueryable<T> query = IDbSet;
            return query;
        }
    }

//Service project

public class HostService
    {
        //HostRepository reqRepo = new HostRepository();
        private UnitOfWork _uow = null;
        protected UnitOfWork UOW
        {
            get
            {
                if (_uow == null)
                {
                    _uow = new UnitOfWork();
                }
                return _uow;
            }
        }         
        public IEnumerable<Host> GetAll()
        {
            return UOW.HostRepository.GetAll();
        }
   }

//WebAPI project

[ResponseType(typeof(HostDTO))]
    [Route("hosts")]
    [HttpGet]
    public async Task<IHttpActionResult> Get()
    {
        IEnumerable<HostDTO> host = hs.GetAll();

        if (host == null)
        {
            return NotFound();
        }

        return Ok(host);
    }
\$\endgroup\$
2
\$\begingroup\$

Entity Framework already implements UoW+Repository for you.

To KISS, your code would simply "wrap" the DbContext with an interface, as explained here, here and here, and possibly elsewhere too. Shortly put: UoW+Repository is an abstraction. Entity Framework is an abstraction. Abstracting an abstraction by wrapping EF with UoW+Repository isn't going to KISS.

That said, there are a number of things to say about your code.


public interface IRepository<T>
{
    IEnumerable<T> GetAll();
}

That seems pretty.. naked. Limited, to say the least. What if you only need to fetch a single T? You're going to GetAll and filter them on the client side with Linq-to-Objects? Linq-to-Entities has much more to offer than that. I'd expect a repository interface to support all CRUD operations!

I don't understand why GenericRepository<T> doesn't implement IRepository<T>. Looks like you can get rid of the HostRepository class


If your entity type is a Host, then this is over-complicated:

var hosts = from host in context.Hosts
            select new Host()
            {
                FirstName = host.FirstName,
                LastName = host.LastName,
                Email = host.Email
            };                
return hosts.AsEnumerable().ToList();

It could be written like this:

return context.Hosts.ToList();

There's no need to project a selection of a given type into ..instances of that same type. There's no need to call AsEnumerable at all to return an IEnumerable<T>, because IQueryable<T> already implements IEnumerable<T>, which means this:

return context.Hosts;

Would return the same thing - the difference is when EF takes your query, translates it into T-SQL and sends it over to the database to materialize the results. You'll probably want to keep Linq-to-Entities within your DAL, which means you'll want to materialize the results before returning from the GetAll method - that's what ToList will do, so returning context.Hosts.ToList() is all you need to do here.


It's not clear why you're using internal, protected and virtual modifiers. A generic repository that needs to be inherited (virtual implies an eventual derived type that will override such a property/method), is missing its purpose I find - the very idea of having a generic repository is to avoid having to implement a repository for every entity type IMO.

\$\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.