2

If there is a class which works as a data access layer, and provides the functionality of CRUD operations for entities, Which version is the best practice when considering performance & Multi threaded environment (i.e methods of this class are called by multiple threads simultaneously . .). . .

Version 1:

DbContext created at class level, shared by all the methods . .

    class EmployeeService{

     private DbContext db=new DbContext();

     public  Employee GetEmployee(int id)

         return db.Employees.Find(id);
     }


     public void AddEmployee(Employee employee){

         db.Employees.Add(employee);
         db.SaveChanges();
     }
}

Version 2:

DbContext for each method call . .

class EmployeeService{

     public  Employee GetEmployee(int id){
        using(DbContext db=new DbContext()){
         return db.Employees.Find(id);
        }
     }


     public void AddEmployee(Employee employee){         
        using(DbContext db=new DbContext()){                 
            db.Employees.Add(employee);
           db.SaveChanges();    
        }
     }
}

UPDATE: May be the question posted is too generic in scope which leads several points to consider.

The point of interest is, the cost of instantiating DbContext object. Can it be created per request (Version 2) or is it heavy weight object and better to create few instances and share them across different calls (Version 1)

CSR
  • 770
  • 1
  • 14
  • 30
  • I'd say none of the above. The way you set it up it is not testable and returning entities of the model is not a good idea. Convert to smaller DTO's based on specific needs. For example, your GetEmployee translate to a SELECT * equivalent in the datastore. Do you really always need that in every method? Pick and improve option 2 is what I would do. – Peter Bons Aug 13 '16 at 12:23
  • V2 is better for multi threading, EF context should never be shared. http://stackoverflow.com/a/6126669/3142139 – M.Hassan Aug 13 '16 at 12:51

2 Answers2

3

There's even a third approach based on manual or automatic dependency injection:

public interface ISomeService 
{
     // Interface members
}

public class SomeService : ISomeService
{
    public SomeService(DbContext dbContext)
    {
         DbContext = dbContext;
    }

    private DbContext DbContext { get; }
}

Then, SomeService won't be the responsible of defining the life-time of injected DbContext, but it's an external class who does it.

That way, your service focuses on doing just what's meant to do (working with the domain and reading/writing data).

Depending on the execution environment, you'll want different DbContext life-styles: per service instance, per request, per thread... There're a lot of choices here depending on the case.

Perhaps you're not considering another scenario: a shared transaction between two or more services. You would need to transfer the responsibility of instantiating DbContext to an upper layer, and then you would inject same DbContext on all participating services, and you would confirm or abandon the whole transaction globally.

Matías Fidemraizer
  • 63,804
  • 18
  • 124
  • 206
  • And then, someone from the outside disposes the context.. now what? At least keep it private. And class level instances of DbContext need to be disposed so you need a IDisposable implementation. That's way you better inject a factory and do a create in each method so the scope stays in the method. – Peter Bons Aug 13 '16 at 13:08
  • @PeterBons Usually you'll expose your services with an interface that's going to be tech-agnostic. You don't have access to the whole injected `DbContext`. In the other hand, why the service should be `IDisposable` if it won't never dispose its associated `DbContext`? – Matías Fidemraizer Aug 13 '16 at 14:01
  • @PeterBons I've made `DbContext` property `private`, BTW I wasn't worried about this because the reason I explain you above... You should never expose a service implementation, but the service interface. – Matías Fidemraizer Aug 13 '16 at 14:03
3

Performance of EF is made up of several factors, scope of the DbContext being one of them.

Some background information regarding the scope is found here: https://msdn.microsoft.com/en-us/data/jj729737.aspx

Scope is not only about performance, it's also about the objects returned. If lazy loading is applied and the DbContext is disposed before you access some navigation properties it will give an exception. See http://www.entityframeworktutorial.net/EntityFramework4.3/lazy-loading-with-dbcontext.aspx

You could write code like this:

public class EmployeeService
{
    public EmployeeDto GetEmployee(int id)
    {
        using(DbContext db=new DbContext())
        {
            return db.Employees.Select(e =>
                new EmployeeDto
                {
                    Id = e.Id,
                    Name = e.Name,
                    Department = e.Department.Name
                }).First(e => e.Id == id);
        }
    }
}

public class EmployeeDto
{
    public int Id { get;set;}
    public string Name { get;set;}
    public string Department { get;set;}
}

So rather than return the whole object you can use projection to limit the set of data that is returned. This you can use to reduce load on the database server as the queries will be less verbose but it also helps to load all the required data before disposing the context. See http://www.entityframeworktutorial.net/querying-entity-graph-in-entity-framework.aspx for some examples of generated queries.

So my advice is to limit the scope of the DbContext. You can inject the DbContext but then you will have no control over the DbContext It will lead to errors like this: http://wallacekelly.blogspot.nl/2012/01/linq-to-entities-objectdisposedexceptio.html

But it all depends on your needs and on what kind of service it is you are building.

Peter Bons
  • 26,826
  • 4
  • 50
  • 74
  • +1 for You can inject the DbContext but then you will have no control over the DbContext It will lead to errors. – CSR Aug 14 '16 at 02:02