Aspnetcore: Consider making public methods of Microsoft.Extensions.DependencyInjection.ActivatorUtilities extension methods

Created on 5 Sep 2018  路  12Comments  路  Source: dotnet/aspnetcore

serviceProvider.CreateInstance(instanceType, params);

feels better than

ActivatorUtilities.CreateInstance(serviceProvider, instanceType, params);

or even

CreateInstance(serviceProvider, instanceType, params);

Most helpful comment

But I'll throw that back at you, @Eilon, why not?

It's basically not a method you want people to use. Adding it as a "first class" (extension) method on IServiceProvider is both kinda confusing (it doesn't match the behavior (or naming) of the rest of the methods) and promotes its usage.

I'd rather see this "hidden" under ActivatorUtilities. If you find yourself calling this method all over the place, it's pretty easy to add your own extension method if it makes it more convenient.

All 12 comments

@paulomorgado thanks for the feedback. Can you provide some info on why you would like this?

I think I already did. It's basically more user friendly. And since it kind of is adding features to IServiceProvider, it makes sense.

But I'll throw that back at you, @Eilon, why not?

But I'll throw that back at you, @Eilon, why not?

It's basically not a method you want people to use. Adding it as a "first class" (extension) method on IServiceProvider is both kinda confusing (it doesn't match the behavior (or naming) of the rest of the methods) and promotes its usage.

I'd rather see this "hidden" under ActivatorUtilities. If you find yourself calling this method all over the place, it's pretty easy to add your own extension method if it makes it more convenient.

I don't want to use it all over the place, but I want to use it where it's useful. Just a thought.

Yeah I think there's value in the API design to make a more solid distinction between regular DI via the service provider, and ActivatorUtilities, which generally should be avoided.

I just crossed the same situation. In my case I wanted to instantiate a job executor (of a scheduled task) and want to get the same comfort as for controllers. Accessing a "Utilities" class in a framework always feels wrong.

Maybe I should handle my job executors as transient services, maybe it is just the name "service" which confuses me. Problem so far is that I cannot late register a transient service.

I'm interested in this one too.

In a situation where I have a class which takes dependencies that are injected by the IServiceProvider but is not in itself a "Service", it feels clunky to have to register dependent classes as services in order to create an instance of them from the service provider. It doesn't feel much better to have to type out ActivatorUtilities.CreateInstance<T>(..) etc. where we'd like to create an object which has dependencies provided by the DI container.

I am looking into using the Microsoft.Extensions.DependencyInjection libraries to replace another DI library in a .NET Core App. (note not ASP.NET Core).

I may have missed something, but it seems like I need to use the ActivatorUtilities.CreateInstance<T>(IServiceProvider) or similar methods on ActivatorUtilities to instantiate an instance of my class.
To me, this would feel a lot more natural as an extension on IServiceProvider.

I've tried to give a concise example below, hopefully it's clear enough.

    public interface ISomeService {}

    public class SomeService : ISomeService {}

    public class SomeObject
    {
        // Takes injected service implementation
        public SomeObject(ISomeService service) {}
    }

    public class Program
    {
        public static void Main(string[] args)
        {
            var services = new ServiceCollection();
            services.AddTransient<ISomeService, SomeService>();

            var provider = services.BuildServiceProvider();

            // I would really like to be able to use an extension method on IServiceProvider like this:
            var fromExtension = provider.CreateInstance<SomeObject>();

            // But in v2.1, I have to do this:
            var fromActivator = ActivatorUtilities.CreateInstance<SomeObject>(provider);

            // Or, I have to create it manually, having resolved required services:
            var service = provider.GetService<ISomeService>();
            var fromConstructor = new SomeObject(service);
        }
    }

Obviously, it is simple to add the extension method for this, but then we'd need to carry it around everywhere, rather than just simply depending on the framework which seems much more natural.

I'd be happy to submit a PR for this if it gets a thumbs up, although the change would be very minimal.

@mdit The same arguments as above still apply though. There is a good reason why this is separated from the service provider. Methods you call on IServiceProvider interact with the service provider and directly resolve services from it.

It would not feel right to have a method on it which does _a lot of things_ that are not resolving dependencies there. ActivatorUtilities in its own is more a equivalent to Activator.CreateInstance which is a reflection-based utility to create objects. ActivatorUtilities.CreateInstance does exactly the same, except that it allows you to _also_ resolve dependencies from a passed service provider.

Creating an instance with the activator utilities is _not_ a functionality of the service provider but rather a reflection-based helper method to create an instance and _comsume_ the service provider. So it makes more sense as an explicit static method.

I consider the proposal of @mdit as an excellent idea. I cannot follow your argumentation, considering all the extension methods cross connecting dozens of things in many other areas of the ASP.NET and Extensions packages. I agree they are different topics but the comfort is definitely in the proposed method. Implemented as extension method is for me a win-win.

Thanks for the feedback, I understand where you are coming from but I can't see any reason why you would use the ActivatorUtilities methods if you don't need to resolve services from the IServiceProvider. As mentioned, you'd just use Activator.CreateInstance. Therefore, since the reason these methods are available seems solely to use a IServiceProvider to resolve arguments while constructing objects, exposing them as extension methods instead still seems appropriate to me.

We don't want it to be first class or making it nice to use this API. The current static method actually behaves a bit differently from the usual ISP.GetService calls and therefore is separate.

@davidfowl You are the boss ;) Is ActivatorUtilities a public API surface?

Was this page helpful?
0 / 5 - 0 ratings