Reputation: 499
I've got this pattern
public abstract class BaseController : Controller
{
readonly RepositoryFactory _rep;
protected RepositoryFactory rep
{
get
{
return _rep;
}
}
readonly ManageRoles _man;
readonly ElementAvailableForUser _env;
protected ElementAvailableForUser env
{
get
{
return _env;
}
}
public BaseController()
: this(DependencyResolver.Current.GetService<RepositoryFactory>(),
DependencyResolver.Current.GetService<ManageRoles>(),
DependencyResolver.Current.GetService<ElementAvailableForUser>()) { }
public BaseController(RepositoryFactory rep, ManageRoles man, ElementAvailableForUser env)
{
_rep = rep;
_man = man;
_env = env;
}
}
so I am able to do something like this
public class HomeController : BaseController
{
public ActionResult Index()
{
return View(rep.Offers.GetAll());
}
public ActionResult Sections()
{
return View(env);
}
}
in all my controller. I'm sure this is antipattern for DI and IoC, so I thin a solution like this
public abstract class BaseController : Controller
{
...
// removed empty constructor
public BaseController(RepositoryFactory rep, ManageRoles man, ElementAvailableForUser env)
{
_rep = rep;
_man = man;
_env = env;
}
}
public class HomeController : BaseController
{
public HomeController(RepositoryFactory rep, ManageRoles man, ElementAvailableForUser env) : base(rep, man, env) { }
...
}
but this solution requires me to insert all dependencies in all controllers and update all constructor if I need a new global variable (like rep) or a new private variable for basecontroller (like man).
Which pattern should I follow and why?
Edit I found this question, and this, but still I can't understand which design patterns should I follow.
Upvotes: 5
Views: 2530
Reputation: 7708
It doesn't seem like you need the BaseController in this case. I think it would be easier (and more testable) to do something like this:
public class HomeController : Controller
{
private readonly IRepository<Offers> _repository;
private readonly RoleManager _roleManager;
private readonly UserManager _userManager;
public HomeController(IRepository<Offers> repository, RoleManager roleManager, UserManager userManager)
{
_repository = repository;
_roleManager = roleManager;
_userManager = userManager;
}
}
Then you could have your IoC container wire up all of those dependencies automatically for each controller.
Upvotes: 3
Reputation: 7131
I don't think with the thin solution you need to always declare rep, man, and env. You could take advantage of default/optional parameters.
public BaseController(RepositoryFactory rep = null, ManageRoles man = null, ElementAvailableForUser env = null)
{
_rep = rep;
_man = man;
_env = env;
}
Then you can use named parameter assignments:
public class HomeController : BaseController
{
public HomeController(ManageRoles man) : base(man: man) { }
...
}
Upvotes: 0