Reputation: 13387
I have many controllers throughout my MVC project and they all have one thing in common. They all have a currentUser property and a currentSettings property. I have done this using DI like so:
[Authorize]
public class _UsersController : Controller
{
#region Fields
private readonly ProfileService service;
private readonly Profile currentUser;
private readonly Settings currentSettings;
#endregion
#region Constructors
public _UsersController()
{
var companyWrapper = new CompanyWrapper();
var profileWrapper = new ProfileWrapper();
var settingsWrapper = new SettingsWrapper();
var companyId = companyWrapper.CurrentCompanyId();
service = new ProfileService(companyId);
currentUser = profileWrapper.CurrentUser();
currentSettings = settingsWrapper.CurrentSiteSettings();
}
#endregion
}
Is there a better way to get the currentUser and current settings to persist? One way I though about doing it would be to have it in a custom AtributeFilter class and store the current user / settings in the session. I just want to get a neat design pattern to fit my needs.
Upvotes: 1
Views: 1003
Reputation: 172826
Step 1: Stop using regions. Seriously, don't use them.
Step 2: Start using the Dependency Injection pattern:
[Authorize]
public class _UsersController : Controller
{
private readonly ProfileService service;
private readonly Profile currentUser;
private readonly Settings currentSettings;
public _UsersController(ProfileService service, Profile currentUser,
Settings currentSettings)
{
this.service = service;
this.currentUser = currentUser;
this.currentSettings = currentSettings;
}
}
We simplified the _UsersController
by passing in (injecting) all dependencies the type actually needs into the constructor. Those wrapper classes are hidden from the _UsersController
, since it shouldn't care about this.
Creating the _UsersController
will now look like this:
var companyWrapper = new CompanyWrapper();
var profileWrapper = new ProfileWrapper();
var settingsWrapper = new SettingsWrapper();
var companyId = companyWrapper.CurrentCompanyId();
var service = new ProfileService(companyId);
var currentUser = profileWrapper.CurrentUser();
var currentSettings = settingsWrapper.CurrentSiteSettings();
new _UsersController(service, currentUser, currentSettings);
And you will have to place this code in a custom controller factory:
public class MyControllerFactory : DefaultControllerFactory
{
protected override IController GetControllerInstance(
RequestContext requestContext, Type controllerType)
{
if (controllerType == typeof(_UsersController))
{
// Create controller here
new _UsersController(service, currentUser, currentSettings);
}
return base.GetControllerInstance(requestContext, controllerType);
}
}
and register that in MVC during startup:
protected void Application_Start()
{
ControllerBuilder.Current.SetControllerFactory(typeof(MyControllerFactory));
}
Step 3: Group related services.
Company, and user information seem highly related. For instance, the current company is probably always the company of the current user. There are probably users of multiple companies using the site at the same time. Site information might be less related, and probably consists of information that is fixed at deployment time. Depending on the situation it might be good to group those classes, for instance, place the company information as property under the Profile
class:
[Authorize]
public class _UsersController : Controller
{
private readonly Profile currentUser;
private readonly Settings currentSettings;
public _UsersController(Profile currentUser, Settings currentSettings)
{
this.currentUser = currentUser;
this.currentSettings = currentSettings;
}
}
The creation code will now start to look like this (the CompanyWrapper
and ProfileService
are now hidden inside the Profile
class:
var profileWrapper = new ProfileWrapper();
var settingsWrapper = new SettingsWrapper();
var currentUser = profileWrapper.CurrentUser();
var currentSettings = settingsWrapper.CurrentSiteSettings();
new _UsersController(currentUser, currentSettings);
Step 4: Start using a dependency injection container:
Creating a custom controller factory just moves the problem and this class will soon become a maintenance nightmare. So instead of writing a your own controller, you can use a dependency injection framework.
Since I'm very used to working with Simple Injector I'll show this framework, but please note that there are a lot of other frameworks (Autofac, Castle Windsor, StructureMap, Ninject) that can do the same.
Instead of writing a custom controller factory we can do the following:
protected void Application_Start()
{
var container = new Container();
container.RegisterMvcControllers(Assembly.GetExecutingAssembly());
container.Register<Profile>(() => new ProfileWrapper().CurrentUser());
container.Register<Settings>(() => new SettingsWrapper().CurrentSiteSettings());
DependencyResolver.SetResolver(new SimpleInjectorDependencyResolver(container));
}
This makes use of the SimpleInjector.MVC3 NuGet package.
Because we registered the Profile
and Settings
classes into the container, it is able to create the _UsersController
on our behalf, simply because it analyzes the constructor arguments of that controller for us and injects its dependencies based on its type information of the constructor arguments.
Nice about this is that you just have to register the Profile
and Settings
once, and the DI framework will inject those types in all controllers (and all other classes in the system) that depend on those services.
Step 5: Apply the proper lifestyle for your registrations.
Since the current user will not change during a single web request, it unsessisarily to create a new ProfileWrapper
and call CurrentUser()
over and over again. That user object can be cached for the duration of that request. The current site settings on the other hand will probably not change for the lifetime of the application, so recreating the SettingsWrapper
and calling CurrentSiteSettings
is just silly.
What we should do is apply the proper caching of these objects. This can be done as follows:
container.RegisterPerWebRequest<Profile>(() => new ProfileWrapper().CurrentUser());
container.RegisterSingle<Settings>(() => new SettingsWrapper().CurrentSiteSettings());
Here we changed the calls from Register
to RegisterPerWebRequest
and RegisterSingle
. The framework will do the rest for us.
Yo dawg. I heard you like patterns, so I put a pattern in your pattern, so you can abstract while you abstract.
Upvotes: 0
Reputation: 1118
You could easily just create a base controller class for your project:
public abstract class ProjectBaseController : Controller
{
#region Fields
protected readonly ProfileService service;
protected readonly Profile currentUser;
protected readonly Settings currentSettings;
#endregion
#region Constructor
public ProjectBaseController()
{
var companyWrapper = new CompanyWrapper();
var profileWrapper = new ProfileWrapper();
var settingsWrapper = new SettingsWrapper();
var companyId = companyWrapper.CurrentCompanyId();
service = new ProfileService(companyId);
currentUser = profileWrapper.CurrentUser();
currentSettings = settingsWrapper.CurrentSiteSettings();
}
#endregion
}
Then just remember to have your inherited classes call the base constructor:
public class _UserController : ProjectBaseController
{
_UserController() : base()
{
// _UserConroller Specific initalization here...
}
}
Upvotes: 2