Skip to main content
replaced http://codereview.stackexchange.com/ with https://codereview.stackexchange.com/
Source Link

I've never TDD'd, but don't do that:

public class MyDataContext<T> : DbContext where T : class

This gives you a context-per-entity, which might work for ultra-simplistic CRUD scenarios, but doesn't scale very well and will quickly give you headaches as soon as you need to deal with more than a single entity type in a single transaction - because that's what a unit-of-work encapsulates: a transaction.

DbContext is a unit-of-work, and IDbSet<T> is a repository; they are an abstraction; by wrapping it with your own, you're making an abstraction over an abstraction, and you gain nothing but complexity.

This blog entry sums it up pretty well. In a nutshell: embrace DbContext, don't fight it.

If you really want/need an abstraction, make your DbContext class implement some IUnitOfWork interface; expose a Commit or SaveChanges method and a way to get the entities:

public interface IUnitOfWork
{
    void Commit();
    IDbSet<T> Set<T>() where T : class;
}

Then you can easily implement it:

public class MyDataContext : DbContext, IUnitOfWork
{
    public void Commit()
    {
        SaveChanges();
    }
}

I don't like IEmployeeService either. This looks like an interface that can grow hair and tentacles and become quite a monster (GetByName, FindByEmailAddress, etc.) - and the last thing you want is an interface that needs to change all the time.

I'd do it something like this, but I'm reluctant to use the entity types directly in the views, I'd probably have the service expose EmployeeModel or IEmployee instead (see this question for more detailsthis question for more details - it's WPF, but I think lots of it applies to ASP.NET/MVC), so as to only have the service class aware of the Employee class, leaving the controller and the view working off some IEmployee implementation, probably some EmployeeModel class, idea being to separate the data model from the domain model.

public class EmployeeService
{
    private readonly IUnitOfWork _unitOfWork;

    public EmployeeService(IUnitOfWork unitOfWork)
    {
        _unitOfWork = unitOfWork;
    }

    IEnumerable<Employee> GetEmployees()
    {
        return _unitOfWork.Set<Employee>().ToList();
    }
}

I've never TDD'd, but don't do that:

public class MyDataContext<T> : DbContext where T : class

This gives you a context-per-entity, which might work for ultra-simplistic CRUD scenarios, but doesn't scale very well and will quickly give you headaches as soon as you need to deal with more than a single entity type in a single transaction - because that's what a unit-of-work encapsulates: a transaction.

DbContext is a unit-of-work, and IDbSet<T> is a repository; they are an abstraction; by wrapping it with your own, you're making an abstraction over an abstraction, and you gain nothing but complexity.

This blog entry sums it up pretty well. In a nutshell: embrace DbContext, don't fight it.

If you really want/need an abstraction, make your DbContext class implement some IUnitOfWork interface; expose a Commit or SaveChanges method and a way to get the entities:

public interface IUnitOfWork
{
    void Commit();
    IDbSet<T> Set<T>() where T : class;
}

Then you can easily implement it:

public class MyDataContext : DbContext, IUnitOfWork
{
    public void Commit()
    {
        SaveChanges();
    }
}

I don't like IEmployeeService either. This looks like an interface that can grow hair and tentacles and become quite a monster (GetByName, FindByEmailAddress, etc.) - and the last thing you want is an interface that needs to change all the time.

I'd do it something like this, but I'm reluctant to use the entity types directly in the views, I'd probably have the service expose EmployeeModel or IEmployee instead (see this question for more details - it's WPF, but I think lots of it applies to ASP.NET/MVC), so as to only have the service class aware of the Employee class, leaving the controller and the view working off some IEmployee implementation, probably some EmployeeModel class, idea being to separate the data model from the domain model.

public class EmployeeService
{
    private readonly IUnitOfWork _unitOfWork;

    public EmployeeService(IUnitOfWork unitOfWork)
    {
        _unitOfWork = unitOfWork;
    }

    IEnumerable<Employee> GetEmployees()
    {
        return _unitOfWork.Set<Employee>().ToList();
    }
}

I've never TDD'd, but don't do that:

public class MyDataContext<T> : DbContext where T : class

This gives you a context-per-entity, which might work for ultra-simplistic CRUD scenarios, but doesn't scale very well and will quickly give you headaches as soon as you need to deal with more than a single entity type in a single transaction - because that's what a unit-of-work encapsulates: a transaction.

DbContext is a unit-of-work, and IDbSet<T> is a repository; they are an abstraction; by wrapping it with your own, you're making an abstraction over an abstraction, and you gain nothing but complexity.

This blog entry sums it up pretty well. In a nutshell: embrace DbContext, don't fight it.

If you really want/need an abstraction, make your DbContext class implement some IUnitOfWork interface; expose a Commit or SaveChanges method and a way to get the entities:

public interface IUnitOfWork
{
    void Commit();
    IDbSet<T> Set<T>() where T : class;
}

Then you can easily implement it:

public class MyDataContext : DbContext, IUnitOfWork
{
    public void Commit()
    {
        SaveChanges();
    }
}

I don't like IEmployeeService either. This looks like an interface that can grow hair and tentacles and become quite a monster (GetByName, FindByEmailAddress, etc.) - and the last thing you want is an interface that needs to change all the time.

I'd do it something like this, but I'm reluctant to use the entity types directly in the views, I'd probably have the service expose EmployeeModel or IEmployee instead (see this question for more details - it's WPF, but I think lots of it applies to ASP.NET/MVC), so as to only have the service class aware of the Employee class, leaving the controller and the view working off some IEmployee implementation, probably some EmployeeModel class, idea being to separate the data model from the domain model.

public class EmployeeService
{
    private readonly IUnitOfWork _unitOfWork;

    public EmployeeService(IUnitOfWork unitOfWork)
    {
        _unitOfWork = unitOfWork;
    }

    IEnumerable<Employee> GetEmployees()
    {
        return _unitOfWork.Set<Employee>().ToList();
    }
}
added 3 characters in body
Source Link
Mathieu Guindon
  • 75.1k
  • 18
  • 191
  • 463

I've never TDD'd, but don't do that:

public class MyDataContext<T> : DbContext where T : class

This gives you a context-per-entity, which might work for ultra-simplistic CRUD scenarios, but doesn't scale very well and will quickly give you headaches as soon as you need to deal with more than a single entity type in a single transaction - because that's what a unit-of-work encapsulates: a transaction.

DbContext is a unit-of-work, and IDbSet<T> is a repository; they are an abstraction; by wrapping it with your own, you're making an abstraction over an abstraction, and you gain nothing but complexity.

This blog entry sums it up pretty well. In a nutshell: embrace DbContext, don't fight it.

If you really want/need an abstraction, make your DbContext class implement some IUnitOfWork interface; expose a Commit or SaveChanges method and a way to get the entities:

public interface IUnitOfWork
{
    void Commit();
    IDbSet<T> Set<T>() where T : class;
}

Then you can easily implement it:

public class MyDataContext : DbContext, IUnitOfWork
{
    public void Commit()
    {
        SaveChanges();
    }
}

I don't like IEmployeeService either. This looks like an interface that can grow hair and tentacles and become quite a monster (GetByName, FindByEmailAddress, etc.) - and the last thing you want is an interface that needs to change all the time.

I'd do it something like this, but I'm reluctant to use the entity types directly in the views, I'd probably have the service expose EmployeeModel or IEmployee instead (see this question for more details - it's WPF, but I think lots of it applies to ASP.NET/MVC), so as to only have the service class aware of the Employee class, leaving the controller and the view working off some IEmployee implementation, probably some EmployeeModel class, idea being to separate the data model from the domain model.

public class EmployeeService
{
    private readonly IUnitOfWork _unitOfWork;

    public EmployeeService(IUnitOfWork unitOfWork)
    {
        _unitOfWork = unitOfWork;
    }

    IList<Employee>IEnumerable<Employee> GetEmployeeListGetEmployees()
    {
        return _unitOfWork.Set<Employee>().ToList();
    }
}

I've never TDD'd, but don't do that:

public class MyDataContext<T> : DbContext where T : class

This gives you a context-per-entity, which might work for ultra-simplistic CRUD scenarios, but doesn't scale very well and will quickly give you headaches as soon as you need to deal with more than a single entity type in a single transaction - because that's what a unit-of-work encapsulates: a transaction.

DbContext is a unit-of-work, and IDbSet<T> is a repository; they are an abstraction; by wrapping it with your own, you're making an abstraction over an abstraction, and you gain nothing but complexity.

This blog entry sums it up pretty well. In a nutshell: embrace DbContext, don't fight it.

If you really want/need an abstraction, make your DbContext class implement some IUnitOfWork interface; expose a Commit or SaveChanges method and a way to get the entities:

public interface IUnitOfWork
{
    void Commit();
    IDbSet<T> Set<T>() where T : class;
}

Then you can easily implement it:

public class MyDataContext : DbContext, IUnitOfWork
{
    public void Commit()
    {
        SaveChanges();
    }
}

I don't like IEmployeeService either. This looks like an interface that can grow hair and tentacles and become quite a monster (GetByName, FindByEmailAddress, etc.) - and the last thing you want is an interface that needs to change all the time.

I'd do it something like this, but I'm reluctant to use the entity types directly in the views, I'd probably have the service expose EmployeeModel or IEmployee instead (see this question for more details - it's WPF, but I think lots of it applies to ASP.NET/MVC), so as to only have the service class aware of the Employee class, leaving the controller and the view working off some IEmployee implementation, probably some EmployeeModel class, idea being to separate the data model from the domain model.

public class EmployeeService
{
    private readonly IUnitOfWork _unitOfWork;

    public EmployeeService(IUnitOfWork unitOfWork)
    {
        _unitOfWork = unitOfWork;
    }

    IList<Employee> GetEmployeeList()
    {
        return _unitOfWork.Set<Employee>().ToList();
    }
}

I've never TDD'd, but don't do that:

public class MyDataContext<T> : DbContext where T : class

This gives you a context-per-entity, which might work for ultra-simplistic CRUD scenarios, but doesn't scale very well and will quickly give you headaches as soon as you need to deal with more than a single entity type in a single transaction - because that's what a unit-of-work encapsulates: a transaction.

DbContext is a unit-of-work, and IDbSet<T> is a repository; they are an abstraction; by wrapping it with your own, you're making an abstraction over an abstraction, and you gain nothing but complexity.

This blog entry sums it up pretty well. In a nutshell: embrace DbContext, don't fight it.

If you really want/need an abstraction, make your DbContext class implement some IUnitOfWork interface; expose a Commit or SaveChanges method and a way to get the entities:

public interface IUnitOfWork
{
    void Commit();
    IDbSet<T> Set<T>() where T : class;
}

Then you can easily implement it:

public class MyDataContext : DbContext, IUnitOfWork
{
    public void Commit()
    {
        SaveChanges();
    }
}

I don't like IEmployeeService either. This looks like an interface that can grow hair and tentacles and become quite a monster (GetByName, FindByEmailAddress, etc.) - and the last thing you want is an interface that needs to change all the time.

I'd do it something like this, but I'm reluctant to use the entity types directly in the views, I'd probably have the service expose EmployeeModel or IEmployee instead (see this question for more details - it's WPF, but I think lots of it applies to ASP.NET/MVC), so as to only have the service class aware of the Employee class, leaving the controller and the view working off some IEmployee implementation, probably some EmployeeModel class, idea being to separate the data model from the domain model.

public class EmployeeService
{
    private readonly IUnitOfWork _unitOfWork;

    public EmployeeService(IUnitOfWork unitOfWork)
    {
        _unitOfWork = unitOfWork;
    }

    IEnumerable<Employee> GetEmployees()
    {
        return _unitOfWork.Set<Employee>().ToList();
    }
}
added generic type constraint to satisfy compiler
Source Link
Mathieu Guindon
  • 75.1k
  • 18
  • 191
  • 463

I've never TDD'd, but don't do that:

public class MyDataContext<T> : DbContext where T : class

This gives you a context-per-entity, which might work for ultra-simplistic CRUD scenarios, but doesn't scale very well and will quickly give you headaches as soon as you need to deal with more than a single entity type in a single transaction - because that's what a unit-of-work encapsulates: a transaction.

DbContext is a unit-of-work, and IDbSet<T> is a repository; they are an abstraction; by wrapping it with your own, you're making an abstraction over an abstraction, and you gain nothing but complexity.

This blog entry sums it up pretty well. In a nutshell: embrace DbContext, don't fight it.

If you really want/need an abstraction, make your DbContext class implement some IUnitOfWork interface; expose a Commit or SaveChanges method and a way to get the entities:

public interface IUnitOfWork
{
    void Commit();
    IDbSet<T> Set<T>(); where T : class;
}

Then you can easily implement it:

public class MyDataContext : DbContext, IUnitOfWork
{
    public void Commit()
    {
        SaveChanges();
    }
}

I don't like IEmployeeService either. This looks like an interface that can grow hair and tentacles and become quite a monster (GetByName, FindByEmailAddress, etc.) - and the last thing you want is an interface that needs to change all the time.

I'd do it something like this, but I'm reluctant to use the entity types directly in the views, I'd probably have the service expose EmployeeModel or IEmployee instead (see this question for more details - it's WPF, but I think lots of it applies to ASP.NET/MVC), so as to only have the service class aware of the Employee class, leaving the controller and the view working off some IEmployee implementation, probably some EmployeeModel class, idea being to separate the data model from the domain model.

public class EmployeeService
{
    private readonly IUnitOfWork _unitOfWork;

    public EmployeeService(IUnitOfWork unitOfWork)
    {
        _unitOfWork = unitOfWork;
    }

    IList<Employee> GetEmployeeList()
    {
        return _unitOfWork.Set<Employee>().ToList();
    }
}

I've never TDD'd, but don't do that:

public class MyDataContext<T> : DbContext where T : class

This gives you a context-per-entity, which might work for ultra-simplistic CRUD scenarios, but doesn't scale very well and will quickly give you headaches as soon as you need to deal with more than a single entity type in a single transaction - because that's what a unit-of-work encapsulates: a transaction.

DbContext is a unit-of-work, and IDbSet<T> is a repository; they are an abstraction; by wrapping it with your own, you're making an abstraction over an abstraction, and you gain nothing but complexity.

This blog entry sums it up pretty well. In a nutshell: embrace DbContext, don't fight it.

If you really want/need an abstraction, make your DbContext class implement some IUnitOfWork interface; expose a Commit or SaveChanges method and a way to get the entities:

public interface IUnitOfWork
{
    void Commit();
    IDbSet<T> Set<T>();
}

Then you can easily implement it:

public class MyDataContext : DbContext, IUnitOfWork
{
    public void Commit()
    {
        SaveChanges();
    }
}

I don't like IEmployeeService either. This looks like an interface that can grow hair and tentacles and become quite a monster (GetByName, FindByEmailAddress, etc.) - and the last thing you want is an interface that needs to change all the time.

I'd do it something like this, but I'm reluctant to use the entity types directly in the views, I'd probably have the service expose EmployeeModel or IEmployee instead (see this question for more details - it's WPF, but I think lots of it applies to ASP.NET/MVC), so as to only have the service class aware of the Employee class, leaving the controller and the view working off some IEmployee implementation, probably some EmployeeModel class, idea being to separate the data model from the domain model.

public class EmployeeService
{
    private readonly IUnitOfWork _unitOfWork;

    public EmployeeService(IUnitOfWork unitOfWork)
    {
        _unitOfWork = unitOfWork;
    }

    IList<Employee> GetEmployeeList()
    {
        return _unitOfWork.Set<Employee>().ToList();
    }
}

I've never TDD'd, but don't do that:

public class MyDataContext<T> : DbContext where T : class

This gives you a context-per-entity, which might work for ultra-simplistic CRUD scenarios, but doesn't scale very well and will quickly give you headaches as soon as you need to deal with more than a single entity type in a single transaction - because that's what a unit-of-work encapsulates: a transaction.

DbContext is a unit-of-work, and IDbSet<T> is a repository; they are an abstraction; by wrapping it with your own, you're making an abstraction over an abstraction, and you gain nothing but complexity.

This blog entry sums it up pretty well. In a nutshell: embrace DbContext, don't fight it.

If you really want/need an abstraction, make your DbContext class implement some IUnitOfWork interface; expose a Commit or SaveChanges method and a way to get the entities:

public interface IUnitOfWork
{
    void Commit();
    IDbSet<T> Set<T>() where T : class;
}

Then you can easily implement it:

public class MyDataContext : DbContext, IUnitOfWork
{
    public void Commit()
    {
        SaveChanges();
    }
}

I don't like IEmployeeService either. This looks like an interface that can grow hair and tentacles and become quite a monster (GetByName, FindByEmailAddress, etc.) - and the last thing you want is an interface that needs to change all the time.

I'd do it something like this, but I'm reluctant to use the entity types directly in the views, I'd probably have the service expose EmployeeModel or IEmployee instead (see this question for more details - it's WPF, but I think lots of it applies to ASP.NET/MVC), so as to only have the service class aware of the Employee class, leaving the controller and the view working off some IEmployee implementation, probably some EmployeeModel class, idea being to separate the data model from the domain model.

public class EmployeeService
{
    private readonly IUnitOfWork _unitOfWork;

    public EmployeeService(IUnitOfWork unitOfWork)
    {
        _unitOfWork = unitOfWork;
    }

    IList<Employee> GetEmployeeList()
    {
        return _unitOfWork.Set<Employee>().ToList();
    }
}
Source Link
Mathieu Guindon
  • 75.1k
  • 18
  • 191
  • 463
Loading