Use TryAdd in AddXxx methods to only add services if not already present. #1182
Conversation
|
/cc @HaoK |
|
As part of this change, you might want to consider using ServiceDescriber instead of a new ServiceCollection which allows you to pass in the configuration instance which would provide service replacement and align EF with the other frameworks AddXXX methods... |
| .AddScoped<IRelationshipListener>(p => p.GetService<NavigationFixer>()) | ||
| .AddScoped<IPropertyListener>(p => p.GetService<ChangeDetector>()); | ||
|
|
||
| serviceCollection.TryAdd(new ServiceCollection() |
There was a problem hiding this comment.
So here, instead of try adding a new service collection that you fill, just directly TryAdd which gets rid of the nested ServiceCollection which looks a bit odd:
var describer = new ServiceDescriber(Configuration);
servicesCollection.AddScoped(your 3 listeners)
servicesCollection.TryAdd(describer.Singleton<>())
There was a problem hiding this comment.
TryAdd can't be chained. It returns bool.
There was a problem hiding this comment.
This is mainly just a nit anyways, but my point was that you can just directly TryAdd the services even without chaining, rather than adding them to a new ServiceCollection and then passing it to the overload of TryAdd that turns around and enumerates the new service collection and adds them one by one...
There was a problem hiding this comment.
Stylistically, I prefer chaining. It would be nice to chain without the additional collection, but given this isn't possible the collection is the way to use chaining with this API.
|
|
…ent. Issue #956 and also see pull request #1129. This change updates all of our AddXxx methods to use TryAdd when adding to the service collection such that only services that are not already registered will be registered. This also means that the code we had to check the service collection for "hosting" services is no longer needed. Note that services for which it is expected that multiple instances can be registered (e.g. listeners and data sources) must not use TryAdd since otherwise only the first registered will be used.
ajcvickers
force-pushed
the
AddXxx1126
branch
from
44bae31 to
a365c55
Compare
Issue #956 and also see pull request #1129.
This change updates all of our AddXxx methods to use TryAdd when adding to the service collection such that only services that are not already registered will be registered. This also means that the code we had to check the service collection for "hosting" services is no longer needed.
Note that services for which it is expected that multiple instances can be registered (e.g. listeners and data sources) must not use TryAdd since otherwise only the first registered will be used.