Injecting dependencies into ASP.NET MVC 3 action filters. What's wrong with this approach?

I'm not positive, but I believe you can just use an empty constructor (for the attribute part) and then have a constructor that actually injects the value (for the filter part) Edit : After a little reading up, it appears that the accepted way to do this is via property injection: public class MyActionFilter : ActionFilterAttribute { Injected public IMyService MyService {get;set;} public override void OnActionExecuting(ActionExecutingContext filterContext) { MyService.DoSomething(); base. OnActionExecuting(filterContext); } } Regarding the why not use a Service Locator question: It mostly just reduces the flexibility of your dependency injection. For example, what if you were injecting a logging service, and you wanted to automatically give the logging service the name of the class it's being injected into?

If you use constructor injection, that would work great. If you're using a Dependency Resolver/Service Locator, you'd be out of luck Update Since this got accepted as the answer, I'd like to go on the record to say that I prefer Mark Seeman's approach because it separates the Action Filter responsibility away from the Attribute. Furthermore, Ninject's MVC3 extension has some very powerful ways to configure action filters via bindings.

See the following references for more details: https://github. Com/ninject/ninject.web. Mvc/wiki/Dependency-injection-for-filters https://github.Com/ninject/ninject.web.

Mvc/wiki/Conditional-bindings-for-filters https://github. Com/ninject/ninject.web. Mvc/wiki/Filter-configurations.

I'm not positive, but I believe you can just use an empty constructor (for the attribute part) and then have a constructor that actually injects the value (for the filter part). * Edit: After a little reading up, it appears that the accepted way to do this is via property injection: public class MyActionFilter : ActionFilterAttribute { Injected public IMyService MyService {get;set;} public override void OnActionExecuting(ActionExecutingContext filterContext) { MyService.DoSomething(); base. OnActionExecuting(filterContext); } } Regarding the why not use a Service Locator question: It mostly just reduces the flexibility of your dependency injection.

For example, what if you were injecting a logging service, and you wanted to automatically give the logging service the name of the class it's being injected into? If you use constructor injection, that would work great. If you're using a Dependency Resolver/Service Locator, you'd be out of luck.

Update Since this got accepted as the answer, I'd like to go on the record to say that I prefer Mark Seeman's approach because it separates the Action Filter responsibility away from the Attribute. Furthermore, Ninject's MVC3 extension has some very powerful ways to configure action filters via bindings. See the following references for more details: https://github.Com/ninject/ninject.web.

Mvc/wiki/Dependency-injection-for-filters https://github. Com/ninject/ninject.web. Mvc/wiki/Conditional-bindings-for-filters https://github.

Com/ninject/ninject.web. Mvc/wiki/Filter-configurations.

Re your comment about the lack of flexibility: Behind the DependencyResolver is an actual IOC container driving it, so you can add any custom logic you want right in there when building up an object. Not sure I follow your point..... – BFree Aug 25 at 15:13 @BFree: When calling DependencyResolver. GetService, the binding method has no idea what class this dependency is being injected into.

What if you wanted to create a different IMyService for certain types of action filters? Or, as I said in my answer, what if you wanted to provide a special argument to the MyService implementation to tell it what class it's been injected into (which is useful for loggers)? – StriplingWarrior Aug 25 at 15:19 OK, I did some messing around, and you're 100% right, there's no way to get the "context" of which the current resolution is happening, so yes that's a downside.

Good point. I would argue though, that adding an Inject attribute is ugly as well though, as that ties your service to an implementation of a particular container as well, where as my DependencyResolver approach does not. I'll leave this question open for a bit, I'm just curious to hear more opinions.

Thanks! – BFree Aug 25 at 15:36 @BFree: I agree that property injection is less-than-ideal. Feel free to leave the question open as long as you like.

I'm interested in hearing other solutions as well. Regarding the Injected attribute, you can specify your own attribute via the NinjectSettings. InjectAttribute property.

This makes it easy to keep your Ninject references contained in your DI setup code. – StriplingWarrior Aug 25 at 15:42.

Yes, there are downsides, as there are lots of issues with IDependencyResolver itself, and to those you can add the use of a Singleton Service Locator, as well as Bastard Injection. A better option is to implement the filter as a normal class into which you can inject whichever services you'd like: public class MyActionFilter : IActionFilter { private readonly IMyService myService; public MyActionFilter(IMyService myService) { this. MyService = myService; } public void OnActionExecuting(ActionExecutingContext filterContext) { if(this.

ApplyBehavior(filterContext)) this.myService.DoSomething(); } public void OnActionExecuted(ActionExecutedContext filterContext) { if(this. ApplyBehavior(filterContext)) this.myService.DoSomething(); } private bool ApplyBehavior(ActionExecutingContext filterContext) { // Look for a marker attribute in the filterContext or use some other rule // to determine whether or not to apply the behavior. } private bool ApplyBehavior(ActionExecutedContext filterContext) { // Same as above } } Notice how the filter examines the filterContext to determine whether or not the behavior should be applied.

This means that you can still use attributes to control whether or not the filter should be applied: public class MyActionFilterAttribute : Attribute { } However, now that attribute is completely inert. The filter can be composed with the required dependency and added to the global filters in global. Asax: GlobalFilters.Filters.

Add(new MyActionFilter(new MyService())).

Thanks for your answer, but no offense, all your links simply point to philosophical arguments about naming of patterns. After reading through all of your answers to other questions you've linked, and to the blog posts, I still don't see any concrete issues that would arise from using my approach. Add to that that now any ActionFilter I use in my application I need to remember to add to the global filters, and I'd argue that this approach could lead to errors of its own.

If you can give a concrete example as to how my approach can lead to errors, then I'll admit I'm being thickheaded :) – BFree Aug 25 at 18:53 3 You asked about what the downsides would be: the downsides are decreased maintainability of your code base, but that hardly feels concrete. This is something that creeps up on you. I can't say that if you do what you propose to do, you will have a race condition, or the CPU may overheat, or kittens will die.

That's not going to happen, but if you don't follow proper design patterns and avoid anti-patterns, your code will rot and four years from now you'll want to rewrite the application from scratch (but your stakeholders won't let you). – Mark Seemann Aug 25 at 19:02 Fair enough. I will agree with you, that if I'm building an all purpose ActionFilter that may be used across projects, then yes, this can be an anti-pattern, as the default constructor may be called, and if DependencyResolver isn't used, you'd get some weird errors.

However 1)your approach still requires registration with the GlobalFilters which can lead to the same issue I've stated above, and 2)What if this action filter isn't uses across applications? What if this action filter is internal? Still as bad?

– BFree Aug 25 at 19:21 In the long run, code based on static state is just harder to maintain. It's global data and we've known for ~40 years that this is bad. There's no way around it... – Mark Seemann Aug 25 at 20:20 2 @BFree: By the way, Remo Gloor has done some fantastic stuff with the MVC3 extension for Ninject.Github.Com/ninject/ninject.web.

Mvc/wiki/… describes how you can use Ninject bindings to define an action filter that gets applied to controllers or actions with a specific attribute on them, rather than having to register the filters globally. This transfers even more control into your Ninject bindings, which is the whole point of IoC. – StriplingWarrior Aug 25 at 22:29.

I'm not positive, but I believe you can just use an empty constructor (for the attribute part) and then have a constructor that actually injects the value (for the filter part).

Yes, there are downsides, as there are lots of issues with IDependencyResolver itself, and to those you can add the use of a Singleton Service Locator, as well as Bastard Injection.

I cant really gove you an answer,but what I can give you is a way to a solution, that is you have to find the anglde that you relate to or peaks your interest. A good paper is one that people get drawn into because it reaches them ln some way.As for me WW11 to me, I think of the holocaust and the effect it had on the survivors, their families and those who stood by and did nothing until it was too late.