3
\$\begingroup\$

In the past I have always called the repositories directly from the controller, but that is a bad practice and now I am implementing a "Business Layer" to my project.

Would I have two UnitOfWorks? One for the Service and then one for the repositories?

BaseController:

public class BaseController : Controller
{
    protected IUnitOfWorkService UnitOfWorkService;
    private readonly IEmployeeService _employeeService;

    protected BaseController(IUnitOfWorkService unitOfWorkService)
    {
        UnitOfWorkService = unitOfWorkService;
        _employeeService = unitOfWorkService.EmployeeService;
    }
}

HomeController:

public class HomeController : BaseController
{
    private readonly IEmployeeService _employeeService;

    public HomeController(IUnitOfWorkService unitOfWorkService) : base(unitOfWorkService)
    {
        _employeeService = unitOfWorkService.EmployeeService;
    }

    public ActionResult Index()
    {
        var emp = _employeeService.GetEmployee(employeeId);
        ...
    }

    ...
}

EmployeeService:

public class EmployeeService : IEmployeeService
{
    private IEmployeeRepository _employeeRepo;

    public EmployeeService(IUnitOfWork unitOfWork)
    {
        _employeeRepo = unitOfWork.EmployeeRepository;
    }

    public Employee GetEmployee(int employeeId)
    {
        return _employeeRepo.GetEmployee(employeeId);
    }

    ...
}

EmployeeRepository

public class EfEmployeeRepository : EfRepository, IEmployeeRepository
{
    public EfEmployeeRepository(SqlContext context) : base(context) { }

    public Employee GetEmployee(int employeeId)
    {
        var employee = Context.Employees
                    .SingleOrDefault(e => e.EmployeeId == employeeId);


        return employee != null ? employee.ToDomain() : null;
    }
}

Unit of Work

IUnitOfWorkService:

public interface IUnitOfWorkService : IDisposable
{
    IEmployeeService EmployeeService { get; }
}

UnitOfWorkService:

public class UnitOfWorkService : IUnitOfWorkService
{
    private readonly IUnitOfWork _unitOfWork;

    private EmployeeService _employeeService;

    public UnitOfWorkService()
    {
        _unitOfWork = new EfUnitOfWork();
    }


    public IEmployeeService EmployeeService
    {
        get { return _employeeService ?? (_employeeService = new EmployeeService(_unitOfWork)); }
    }

    public void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }

    protected virtual void Dispose(bool disposing)
    {
        if (disposing)
            _unitOfWork.Dispose();
    }
}

IUnitOfWork:

public interface IUnitOfWork : IDisposable
{
    IEmployeeRepository EmployeeRepository { get; }
}

UnitOfWork:

public class EfUnitOfWork : IUnitOfWork
{
    private readonly SqlContext _context;

    private EfEmployeeRepository _employeeRepository;

    public IEmployeeRepository EmployeeRepository
    {
        get { return _employeeRepository ?? (_employeeRepository = new EfEmployeeRepository(_context)); }
    }

    public EfUnitOfWork()
    {
        _context = new SqlContext();
    }

    public void Save()
    {
        _context.SaveChanges();
    }

    public void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }

    protected virtual void Dispose(bool disposing)
    {
        if (disposing)
            _context.Dispose();
    }
}

Register Dependencies

builder.RegisterType<EfUnitOfWork>().As<IUnitOfWork>().InstancePerRequest();
builder.RegisterType<UnitOfWorkService>().As<IUnitOfWorkService>().InstancePerRequest();
\$\endgroup\$
  • \$\begingroup\$ do you need 2 UoW's? \$\endgroup\$ – Malachi Aug 13 '15 at 15:59
  • \$\begingroup\$ Other examples I look at use the generic repository pattern and it looks like they only have 1 UoW. I haven't seen anyone else implement a UoW for the Service and another one for the repository. The alternative would be to have the controllers accept each service as a parameter and the UoW would be in the service? \$\endgroup\$ – Andrew Aug 13 '15 at 16:01
1
\$\begingroup\$
public class BaseController : Controller
{
    protected IUnitOfWorkService UnitOfWorkService;
    private readonly IEmployeeService _employeeService;

    protected BaseController(IUnitOfWorkService unitOfWorkService)
    {
        UnitOfWorkService = unitOfWorkService;
        _employeeService = unitOfWorkService.EmployeeService;
    }
}
public class HomeController : BaseController
{
    private readonly IEmployeeService _employeeService;

    public HomeController(IUnitOfWorkService unitOfWorkService) : base(unitOfWorkService)
    {
        _employeeService = unitOfWorkService.EmployeeService;
    }

    public ActionResult Index()
    {
        var emp = _employeeService.GetEmployee(employeeId);
        ...
    }

    ...
}  
  • I don't see a reason here to have private readonly IEmployeeService _employeeService; in the HomeController. It would be better to make the _employeeService variable of the BaseController protected so it can be used by the objects which inherits the BaseController.

  • Why is the protected IUnitOfWorkService UnitOfWorkService; not readonly ?


Style

  • Using abbreviations or naming variables is bad practice. Does it hurt to name the variable IEmployeeRepository _employeeRepo like IEmployeeRepository _employeeRepository ?

  • It does not hurt to use braces {} for single if statements but will make the code less error prone. I would like to encourage you to always use them.

\$\endgroup\$
  • \$\begingroup\$ I'm not sure I agree with making _employeeRepository part of the base. Seems to be tying up a base to this field making all inherited classes have it even if they don't want it. If this was needed I would consider having a BaseEmployeeController but that starts down a slippery deep inheritance tree setup. \$\endgroup\$ – dreza Aug 14 '15 at 9:01
  • \$\begingroup\$ @dreza I didn't say _employeeRepository but _employeeService which already is part of the BaseController. \$\endgroup\$ – Heslacher Aug 14 '15 at 9:03
  • \$\begingroup\$ @Heslacher Having the _employeeService as part of the BaseController is a good idea, since every controller will need that anyways. I forgot to mark the protected IUnitOfWorkService as readonly. I'll get that updated. I agree with you that abbreviations are bad practice and I usually avoid them, but I didn't see any problems with "repo." I don't think anyone would have problems trying to figure out what that stands for. \$\endgroup\$ – Andrew Aug 14 '15 at 12:11

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.