Create instance of interface with dependency injection

Question

I have a base controller and before every page load I want to get the current user. I originally had a constructor in my BaseController that looked like this

public BaseController(ISystemUserCommand command)
{
    _systemUserCommand = command
}

The problem with this then is that every controller that inherits from the BaseController would have to contain the ISystemUserCommand in its constructor, which I don't think would be good.

Instead I tried to create just an instance of the service class (shown below - it's the commented line under var sid...) but I need to pass in user service. How would I pass in the user service here or is this a bad way of doing it?

public abstract class BaseController : Controller
{
    public SystemUserViewModel CurrentUser { get; set; }

    private readonly ISystemUserCommand _systemUserCommand;

    public SystemUserViewModel GetCurrentUser()
    {
        if (HttpContext == null || HttpContext.User == null) return null;
        if (CurrentUser != null) return CurrentUser;

        var sid = System.Web.HttpContext.Current.Request.LogonUserIdentity.User.ToString();

        //var command = new SystemUserCommand();

        CurrentUser = _systemUserCommand.GetUser(sid);

        return CurrentUser;
    }

    public void SetUserInformation(SystemUserViewModel currentUser)
    {
        ViewBag.UserId = currentUser.SystemUserId;
        ViewBag.FullName = string.Format("{0} {1}", currentUser.FirstName, currentUser.LastName);
        ViewBag.FirstName = currentUser.FirstName;
        ViewBag.LastName = currentUser.LastName;
        ViewBag.CurrentUser = currentUser;
    }

    protected override void OnActionExecuting(ActionExecutingContext filterContext)
    {
        var currentUser = GetCurrentUser();

        if (currentUser != null)
        {
            if (currentUser.IsActive)
            {
                SetUserInformation(currentUser);
            }
            else
                filterContext.Result = RedirectToAction("denied", "unauthorized");
        }
        else
            filterContext.Result = RedirectToAction("denied", "unauthorized");

        base.OnActionExecuting(filterContext);
    }
}

public class SystemUserCommand : ISystemUserCommand
{
    private readonly ISystemUserBusiness _systemUserBusiness;

    public SystemUserCommand(ISystemUserBusiness systemUserBusiness)
    {
        _systemUserBusiness = systemUserBusiness;
    }

    ...
}

All of your issues stem from the fact that you are using a base controller for the singular purpose of sharing code. While there may be reasons for making a base controller in MVC, sharing code is certainly not one of them because it tightly couples the rest of your application to that base class. As was pointed out by Ilya, filters can be used as a loosely-coupled alternative. See this answer for a more complete explanation. — NightOwl888
– NightOwl888, Commented Oct 30, 2016 at 13:13

Sean Sobey · Accepted Answer · 2016-10-28 18:27:44Z

2

You could use property injection instead of constructor injection, via the base class, eg using unity:

public abstract class BaseController : Controller
{
    [Dependency]
    public ISystemUserCommand SystemUserCommand { get; set; }
}

This would mean the interface reference is only on the base class.

See here for the full examples.

EDIT, Autofac example:

You don't need property attributes on the dependency,

public abstract class BaseController : Controller
{
    public ISystemUserCommand SystemUserCommand { get; set; }
}

Just to register the properites to auto resolve on the autofac builder:

builder.RegisterControllers(typeof(MvcApplication).Assembly).Where(t => t.IsAssignableFrom(typeof(BaseController))).PropertiesAutowired();

See autofac property injection here.

edited Oct 28, 2016 at 18:27

answered Oct 28, 2016 at 17:35

Sean Sobey

9927 silver badges13 bronze badges

Sign up to request clarification or add additional context in comments.

4 Comments

Andrew Over a year ago

Do you have an example with AutoFac?

Sean Sobey Over a year ago

I added an autofac example :)

Andrew Over a year ago

Does this still allow constructor injection or does everything default to property injection?

Sean Sobey Over a year ago

I added a way to scope the auto property injection and a link to the autofac dos as well.

Ilya Chernomordik · Accepted Answer · 2016-10-28 17:53:23Z

1

First of all, it does not seem a good idea to have OnActionExecuting override in the controller. You can use filters, that are specially designed for this purpose. And it seems that is the main reason you created the BaseController at all.

Regarding the problem with injecting the system command in all the required service, I would do so, but without inheriting from a base class, since I generally prefer aggregation to inheritance. That would mean that each controller that needs to work with the service will get it.

Another option that I have used few times to abstract some operations is to create a UserSerivce that will provide the required operations to the controllers. It will have ISystemUserCommand and HttpContext injected inside so that all of your controllers won't have to do the job. You can either use HttpContext.Current as static or abstract it away if you need testability.

Moreover I would not recommend property injection since it is more obscure than constructor injection that should be preferred if possible.

You can read more about filters here. Unfortunately if you use filters it's not that easy to inject in filters themselves and mostly done with property injection or ServiceLocator pattern (which is not good usually). It's possible to do better with some amount of voodoo though. I think that SimpleInjector has a lot of examples and tutorials on how to apply DI to filters in MVC, maybe they even have a nuget package now to ahieve that.

edited Oct 28, 2016 at 17:53

answered Oct 28, 2016 at 17:44

Ilya Chernomordik

30.9k31 gold badges148 silver badges245 bronze badges

16 Comments

Andrew Over a year ago

My base controller does other stuff too, I just didn't include it. I put OnActionExecuting here because I want to populate viewbags for the layout page with user information. You think I should use a filter instead for that?

Ilya Chernomordik Over a year ago

If you show some common data on your page (like user info) I would rather create a special partial view that would be reused in MVC pages. This is very non-obvious operation and others might think it appears "magically" until they understand how it works.

Andrew Over a year ago

Hmm okay. So if I put someone's name and company in the navbar of the page I would move that to a partial view and then I'd create a filter to populate the partial?

Ilya Chernomordik Over a year ago

No, not quite. You would put that navbar to a partial view and you will inject your command/service in the controller that is responsible for that view. Then you would put your partial view in the required place in your pages. Filter you need to use for authrorization (I see you do RedirectToAction now). And you can inject the same UserService in the filter since you would need to do similar operation there (get the user).

Andrew Over a year ago

Okay, so as an example in my HomeController I would have HomeController(ISystemUserCommand command) and inside the constructor would I would call GetUser(int id) and populate the viewbags that way?

|

Collectives™ on Stack Overflow

Create instance of interface with dependency injection

2 Answers 2

4 Comments

16 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

4 Comments

16 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related