SimpleInjector icon indicating copy to clipboard operation
SimpleInjector copied to clipboard

Add Container.Register(Type, Assembly, TypesToRegisterOptions) overload(s)

Open dotnetjunkie opened this issue 5 years ago • 2 comments

Batch registration sometimes requires to call the GetTypesToRegister method, which can be a hassle and this method is hard to find. Instead of using GetTypesToRegister this could be integrated in the Register method by adding one or multiple overloads that accept a TypesToRegisterOptions argument.

So instead of doing this:

var types =
    container.GetTypesToRegister(typeof(IRequestHandler<,>), assemblies,
        new TypesToRegisterOptions
	{
		IncludeGenericTypeDefinitions = true
	});

container.Register(typeof(IRequestHandler<,>), types);

We could do this:

container.Register(typeof(IRequestHandler<,>), assemblies, new TypesToRegisterOptions
{
	IncludeGenericTypeDefinitions = true
});

dotnetjunkie avatar Feb 08 '20 11:02 dotnetjunkie

feature-799 branch added.

dotnetjunkie avatar Apr 15 '20 09:04 dotnetjunkie

Although the previous example makes sense as a way to simplify a not that uncommon scenario, the use of TypesToRegisterOptions might not be the best pick, because it allows a user to do the following:

container.Register(typeof(IRequestHandler<,>), assemblies, new TypesToRegisterOptions
{
    IncludeDecorators = true
});

The TypesToRegisterOptions includes an IncludeDecorators property. This is useful when calling GetTypesToRegister because it allows selecting found decorators using reflection. However, it never makes sense to call Container.Register with IncludeDecorators = true, because Register will in that case try to register the decorator as a 'normal' type, instead of through calling RegisterDecorator. This either causes the call to Register to fail, or it will lead to failure during Verification.

So, instead, it would be better to create an overload that accepts a different type, e.g.:

container.Register(typeof(IRequestHandler<,>), assemblies, new BatchRegistrationOptions
{
	IncludeGenericTypeDefinitions = true
});

I'm, however, a bit hesitant to introduce a new type for this, because in that case the design should be thought through a bit more:

  • What would be the ideal name be?
  • Would Container.Register and Container.Collection.Register both need that type? Or does Collection.Register need a different type that doesn't include the IncludeComposites property?
  • And if a different type is needed, what should the name be of both types?
  • Would this API simplification be worth it? Do enough people struggle with this scenario?

dotnetjunkie avatar Apr 15 '20 09:04 dotnetjunkie