4

I'm starting a new small project with ASP.NET MVC and Entity Framework. (SQL Server - around 20 DB tables) In past projects I’ve used Linq2SQL but it seems to be obsolete.

I've read a lot of posts on using repository pattern for EF (pros and cons) , For me it seems better/simpler to code without repository pattern.

I created the following project architecture :

namespace MySite.Models
{
    public class User
    {
        public Int32 ID { get; set; }
        public String Email { get; set; }
        public String Password { get; set; }
        public String Name { get; set; }
        public Int32 Gender { get; set; }
    }
}

namespace MySite.DAL
{
    public class Users
    {
       public static IEnumerable<User> GetUsers()
        {
            using (var context = new DatingSiteContext())
            {
                return context.Users.ToList();
            }
        }

        public static User GetUserByID(int id)
        {
            using (var context = new DatingSiteContext())
            {
                return context.Users.Find(id);
            }
        }
}

namespace MySite.Controllers
{
    public class HomeController : Controller
    {

        public ActionResult Index()
        {
            ViewBag.Message = "Modify this template to jump-start your ASP.NET MVC application.";

            var users = DAL.Users.GetUsers();

            return View(users);
        }
    }
}
  • What are the disadvantage of using EF like this? (Except lack of unit testing support)
  • Is it wrong to create a new DbContext on each call to DAL ? Any Performance hit?
  • Any other recommended structure for using EF ? Examples? :)
  • Would you use Linq2SQL in a new project ?

Thank you.

Edit:

The code inside GetUsers() and GetUserByID() is just for example , i understand its a bad practice to return all records from the db (paging or filter in missing)

RuSh
  • 1,643
  • 7
  • 25
  • 39
  • For me your DAL is kind of like repositories just you added the name "Users" instead of "UserRepository" – dvjanm Oct 17 '13 at 13:17
  • Keep in mind calling `ToList()` like you do in `return context.Users.ToList();` will return the entire dataset into memory which may take a performance hit if there are a lot of users. – alstonp Oct 17 '13 at 13:23
  • FYI, if Linq2Sql works for you, keep using it. It's not going away, it's just not getting any new features. For a small project, you should be fine. Also, EF dbcontexts are intended to be short lived, you can run into problems if you use them in prolonged situations, so yes.. DO create a new one every time you use it. – Erik Funkenbusch Oct 17 '13 at 14:49

3 Answers3

7

You actually just created a repository only you call it a 'data access layer' which is, in my opinion, not a good name since Entity Framework is the data access layer. A repository is an abstraction on top of a data access layer, Entity Framework in this case.


Is it wrong to create a new DbContext on each call to DAL ? Any Performance hit?

Nope, it's just fine, but it might cause trouble when you fetch an entity in one instance of DbContext, and try to update it in another instance.


Would you use Linq2SQL in a new project ?

Nope, Microsoft proposed Entity Framework as the successor of L2SQL and active development of it has stopped.


Any other recommended structure for using EF ? Examples? :)

The approach you use, specific repositories, will result in a lot of redundant code. You could create a generic repository implementing an interface:

public interface IRepository<TEntity> 
    where TEntity : class, new()
{
    IEnumerable<TEntity> GetAll();

    TEntity GetById(int id);

    IQueryable<TEntity> Table { get; }
}

And an implementation of this:

public EfRepository<TEntity> : IRepository<TEntity>
    where TEntity : class, new()
{
    private readonly DatingSiteContext _context;

    public EfRepository()
    {
        _context = new DatingSiteContext();
    }

    private IDbSet<TEntity> Entities
    {
        get
        {
            return _context.Set<TEntity>();
        }
    }

    public IEnumerable<TEntity> GetAll()
    {
        return Entities.ToList();
    }

    public TEntity GetById(int id)
    {
        return Entities.Find(id);
    }

    public IQueryable<TEntity> Table 
    { 
        get { return Entities; }
    }   
}

You can use this repository in your controller like this:

public class HomeController : Controller
{
    private readonly IRepository<User> _userRepository;

    public HomeController()
    {
        _userRepository = new EfRepository<User>();
    }

    public ActionResult Index()
    {
        var users = _userRepository.GetAll();
        var inactiveUsers = _userRepository.Table.Where(u => !u.Active).ToList();
    }
}

This generic repository allows you to create mocked repositories:

public class FakeUserRepository : IRepository<User> 
{ 
    // ... 
}


This approach might seem like a lot of code, but as your entity type amount grows, it will save you a lot of work since all you have to do is create an IRepository<> field in a controller. Yet you have a lot of flexibility with the IQueryable<> property which allows deferred execution.

I'm not saying this is the best method, just one I use regularly in projects. I have to say that I usually write a business (service) layer between the controller and the repositories. I keep my business logic and complex Linq queries (and their execution) there. I also use an IoC container which handles the lifetime of my objects (instances of DbContext and services for example). See this question for more information about that.

Community
  • 1
  • 1
Henk Mollema
  • 44,194
  • 12
  • 93
  • 104
  • I would rephrase that. A Repository is an abstraction on top of a data access layer, not necessarily an ORM. – Erik Funkenbusch Oct 17 '13 at 14:51
  • Thanks for detailed response :) 1. You wrote "might cause trouble when fetch an entity in one instance of DbContext, and update it in another instance.” Does the generic repository described above help solve this problem? 2. I tried using Generic interface for the repositories but I found that 90% of the time my queries are a lot more specific/complex and I just don’t use the generic GetByID or GetAll , would you still see a point in created a generic interface? 3. In the generic class you created the DbContext once per repository, why is that? Would that require using dispose function? – RuSh Oct 17 '13 at 15:01
  • @sharru - There is a difference between a generic repository and a repository. Generic repository is often the basis for a specialized one. You can build upon it. – Erik Funkenbusch Oct 17 '13 at 15:05
  • I noticed that you write additional Linq code (to get inactiveUsers) in the controller itself using the Table functions provided by the generic interface. What would you do if you Linq query becomes complex ? Would you still keep the Linq code inside the controller or create a dedicated UserRepository Class ? – RuSh Oct 17 '13 at 15:08
  • @sharru 1. It does if you keep using the same instance of the repository, but not if you use two different repositories. 2. If you really don't use them, then there is not really a point in this indeed. 3. You can also create it once per method and wrap it inside a using statement indeed, but that does not allow you to use deferred execution. You should indeed add a `Dispose` method which you can call when your controller disposes (override `Dispose` in your controller). I usually use an IoC container which handles the lifetime of objects, but that might be too much for a small project. – Henk Mollema Oct 17 '13 at 16:33
  • @sharru if you write a lot of lot of the same queries in your controller, you could indeed create a specific repository and place it there. I usually create a business (service) layer between my controllers and repositories where I compose the Linq queries. But again, that might be an overkill for a small project. I've also updated my answer with some more information. – Henk Mollema Oct 17 '13 at 16:34
1

My thoughts

Whats the disadvantages:

  • You cant really unit test anywhere that uses the static methods you have defined in your DAL.
  • They are also strongly coupled making them more difficult to swap out at runtime, if that became a requirement.
  • You may start to get additional complications if you need to commit several updates in a transaction

Is it wrong to create a new DbContext on each call?

  • No, this is fine. The DbContext is lightweight and meant to be used this way.

Other patterns

  • You already mentioned the repository pattern which is pretty solid, especially when used with a unit of work pattern.

Would you use Linqtosql

  • No - Linqtosql is pretty much done with, entity framework provides a more complete and generally better solution to this problem
Matt Whetton
  • 6,616
  • 5
  • 36
  • 57
0

I would rethink how you implemented GetUsers(). You are calling ToList() which will cause all the rows from the underlying table to be returned and stored in memory. If the tables grows large enough you will run into performance issues. It's better to return an IQueryable<User> instead and have your method return context.Users.

Of course you'll run into the problem that the context has already been disposed by the time you execute the IQueryable<>, so you'll need to handle the life cycle of the context in a different way.

If the project is small enough, then you can just store an instance of the Context at the Controller level, and dispose of it when the controller is being disposed. If you do that, make sure you don't do anything in your views that would cause additional queries to be executed (e.g. access a collection off of User if one exists) or else that will error out.

Brian Ball
  • 12,268
  • 3
  • 40
  • 51