ServiceCollectionDescriptorExtensions.Add extension method not bound

[peter-perot]

The compiler does not bind the extension method ServiceCollectionDescriptorExtensions.Add(this IServiceCollection collection, ServiceDescriptor descriptor) when the method is called on an instance of type IServiceCollection. This is the correct compiler behavior, since IServiceCollection derives from IList<ServiceDescriptor>, and IList<> has an interface definition for Add.

On the other hand, when calling Add on an instance of type ServiceCollection (the concrete class, not the interface), the compiler binds the extension method. This is due to the fact that ServiceCollection uses explicit interface method implementation for the Add method.

This is correct so far, but I wonder what is the point for the extension method?

• It is not bound on IServiceCollection instances.
• It is bound on ServiceCollection instances.
• It adds a null-check, which is missing in the implemention of ServiceCollection.

Wouldn't it be better to put the null-check into the class implementation?

An alternative is to rename the extension method to something like AddDescriptor or AddService.

[analogrelay]

It does seem like this extension isn't useful. Since it's somewhat harmless we probably won't get to this in 3.0 but I'll label as help wanted because we'd look at a PR here.

Moving the null check to ServiceCollection is probably reasonable since I don't know what it would mean to register a a null descriptor anyway :).

[SARAVANA1501]

@anurse I would like to contribute this, but before begin, Add method in ServiceCollection.cs is returns void, so We cant do method chaining. But Add method in ServiceCollectionDescriptorExtensions returns ServiceCollection so it enable us to further chain the other method. Is this a concern?

[peter-perot]

Ah, I see, the original intent of the extension method was possibly to add chaining, but it doesn't work, as we can see.

We could overcome this issue with different method names. When chaining comes into play, a typical naming convention is to use the "With" prefix.

So, we could name the extension methods this way:

• With
• WithScoped
• WithSingleton

and so on.

On the othen hand I don't understand why chaining was not implemented with the TryAdd methods? They simply return void (not even bool).

For consistency I would also propose the following method names when chaining comes into play:

• TryWith
• TryWithScoped
• TryWithSingleton

But I think this should decide someone who has a pretty good overview of the conventions used in .NET Core.

As part of the migration of components from dotnet/extensions to dotnet/runtime (aspnet/Announcements#411) we will be bulk closing some of the older issues. If you are still interested in having this issue addressed, just comment and the issue will be automatically reactivated (even if you aren't the author). When you do that, I'll page the team to come take a look. If you've moved on or workaround the issue and no longer need this change, just ignore this and the issue will be closed in 7 days.

If you know that the issue affects a package that has moved to a different repo, please consider re-opening the issue in that repo. If you're unsure, that's OK, someone from the team can help!

[peter-perot]

Anybody around who knows the initial intent of the extension method? If it was chaining, then we should not use Add as a method name as I already noted.

Paging @dotnet/extensions-migration ! This issue has been revived from staleness. Please take a look and route to the appropriate repository.

Tagging subscribers to this area: @eiriktsarpalis
Notify danmosemsft if you want to be subscribed.