New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should Container.Collections be renamed to Container.Collection? #556

Closed
dotnetjunkie opened this Issue May 13, 2018 · 2 comments

Comments

2 participants
@dotnetjunkie
Collaborator

dotnetjunkie commented May 13, 2018

Simple Injector v4.1 introduced the Container.Collections property to group collection-based registrations in #517. From a readability standpoint, however, it might make more sense to rename this property to Container.Collection. Take the following example for instance:

container.Collections.Register<ILogger>(typeof(ILogger).Assembly);

container.Collections.Append<ILogger, FileLogger>();
container.Collections.Append<ILogger, SqlLogger>();

These Register and Append methods however do not register or append collections, but actually a single collection. The following, therefore, does make more sense:

container.Collection.Register<ILogger>(typeof(ILogger).Assembly);

container.Collection.Append<ILogger, FileLogger>();
container.Collection.Append<ILogger, SqlLogger>();

If the change is made, it should be made with the introduction of v4.3, because in the v4.2 and v4.1 releases, the Collections grouper only contains a few little used Append and Create methods, while with #542 in v4.3, RegisterCollection is moved to the grouper, which means a much larger group of developers will start to use the new syntax. Therefore, making the change after v4.3 is of the table.

@dotnetjunkie

This comment has been minimized.

Collaborator

dotnetjunkie commented May 14, 2018

To minimize the pain, I could even imagine to push this change to 4.1.3 and 4.2.3 as well.

@TheBigRic

This comment has been minimized.

Collaborator

TheBigRic commented May 15, 2018

I totally agree. Naming this collection of methods Collection instead of Collections is far better and should be done now, because after this point the breaking change becomes way too big.

dotnetjunkie added a commit that referenced this issue May 15, 2018

dotnetjunkie added a commit that referenced this issue May 15, 2018

dotnetjunkie added a commit that referenced this issue May 15, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment