search

dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.17k stars 4.72k forks source link

Change TryAdd IServiceCollection extensions to return "something" #36492

Open DanielBryars opened 5 years ago

DanielBryars commented 5 years ago

Is your feature request related to a problem? Please describe.

All the Try variants of the Add, AddTransient, AddSingleton etc return void: Source.

This means I can't chain the methods when building my dependencies, like I can with AddTransient<> etc.

This means it breaks the standard pattern TryXXX which returns a bool if it can't do what it's trying.

Returning void is the worst of both worlds.

Describe the solution you'd like

Return IServiceCollection so that you can do the fluent method chaining.

Describe alternatives you've considered

Return a bool. I don't like this, because there's very little you'd do with the answer.

Additional context

See my question on so for some more discussion.

grahamehorner commented 5 years ago

I would like to see an optional out bool added to the Try methods, that allows later evaluating the result if needed as well as the return type change to IServiceColletion. I'm not that bothered if the extension methods names differ slightly, but do agree flunte chain would be good.

ghost commented 4 years ago

As part of the migration of components from dotnet/extensions to dotnet/runtime (https://github.com/aspnet/Announcements/issues/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!

khellang commented 4 years ago

This is kinda funny. To quote myself from https://github.com/aspnet/DependencyInjection/pull/353#issuecomment-178206877 and https://github.com/aspnet/DependencyInjection/issues/447#issuecomment-248633002:

Trust me - this'll come up time and time again :wink:

I doubt anything will happen here 😅

ghost commented 4 years ago

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