Remove the Add* methods from (I)ServiceCollection that don't take in an IServiceDescriptor

[halter73]

All of the Add* methods in ServiceCollection that do not take in an IServiceDescriptor (e.g. AddTransient) can be implemented as extension methods in ServiceCollectionExtensions instead of in the ServiceCollection itself. Generally we prefer extension methods over regular methods all else being equal.

[Eilon]

Generally we prefer extension methods over regular methods all else being equal.

Is that true? That just seems to make Intellisense worse to me, no? An extraneous class shows up to contain the extension methods, plus the methods in Intellisense get a tiny little distracting adornment.

Now, in the case of interfaces, we definitely don't want to add non-essential members and instead use extension methods. But for classes, I don't see the reasoning.

[moozzyk]

+1
In addition extension methods are static methods which oftentimes makes testing/mocking very hard (vide SignalR client)

[pranavkm]

Perhaps the "in general" statement is not necessarily true. However in this case methods such as AddTransient, AddScoped etc are sugar over a set of methods that add to the ServiceCollection. Additionally since we expect the implementation of Add* to call into Add(IServiceDescriptor) and they aren't meant to have identical behavior across all implementations of IServiceCollection. Extension methods make sense in this scenario

[Eilon]

I'm not sure that "sugar" status should affect whether a method is an extension method or not. If the extension method applies to only a concrete class, writing an extension method seems negative in every possible way. It's more work for us, more confusing to users, harder to follow code, etc.

[pranavkm]

These methods are on the IServiceCollection interface - https://github.com/aspnet/DependencyInjection/blob/dev/src/Microsoft.Framework.DependencyInjection/IServiceCollection.cs. I think the title of the work item is misleading

[Eilon]

Oh, so the bug title and text are incorrect? Is this just about the interface methods?

[pranavkm]

From our conversation with @loudej it did seem like we were talking about changing the interface and subsequently changing the concrete implementation to not take a IConfiguration. I'll let @halter73 confirm this

[halter73]

I updated the title to reflect that the extraneous Add* methods are currently on the IServiceCollection interface.

Right now we have a weird distribution of Add* methods. Many are on the IServiceCollection interface, but even more are in ServiceCollectionExtensions. After talking to @lodejard with @pranavkm last night, we decided it was best to make all the Add* methods we could extension methods.

I went too far with my "generally we prefer extension methods" comment. However, as you point out @Eilon, this statement makes sense for interfaces. It makes the interfaces easier to implement/mock, and it makes it less likely we will need to change the interface which is especially important since it is assembly neutral.

[Eilon]

OK then I like it more 😄