dadhi / DryIoc

DryIoc is fast, small, full-featured IoC Container for .NET
MIT License
982 stars 122 forks source link

Keep the _implementationTypeOrProviderOrPubCtorOrCtors field private #613

Closed yallie closed 7 months ago

yallie commented 7 months ago
  1. Move the first part of the FactoryMethod.MostResolvableConstructor method to ReflectionFactory.GetConstructors.
  2. Remove the (almost) the same duplicate code portion from FactoryMethod.Constructor method.
  3. Make ReflectionFactory.WithTypeProvider class override virtual GetConstructors method with Func<​Type> logic.
yallie commented 7 months ago
Obsolete By the way, there is a tiny bit of inconsistency [here](https://github.com/dadhi/DryIoc/blob/master/src/DryIoc/Container.cs#L6758): ```charp private static FactoryMethod MostResolvableConstructor(Request request, BindingFlags additionalToPublicAndInstance = 0, Func condition = null) { var ctorsOrCtorOrType = ((ReflectionFactory)request.Factory)._implementationTypeOrProviderOrPubCtorOrCtors; ConstructorInfo[] ctors = null; if (ctorsOrCtorOrType is ConstructorInfo ci) { if (additionalToPublicAndInstance == 0) return condition == null || condition(ci.DeclaringType, ci.GetParameters()) ? new FactoryMethod(ci) : null; ctors = ci.DeclaringType.GetConstructors(BindingFlags.Public | BindingFlags.Instance | additionalToPublicAndInstance); } else if (ctorsOrCtorOrType is ConstructorInfo[] cs) ctors = cs; else if (ctorsOrCtorOrType is Type t) ctors = t.GetConstructors(BindingFlags.Public | BindingFlags.Instance | additionalToPublicAndInstance); else if (ctorsOrCtorOrType is Func typeProvider && typeProvider() is Type it) ctors = it.GetConstructors(BindingFlags.Public | BindingFlags.Instance | additionalToPublicAndInstance); else Throw.It(Error.ImplTypeIsNotSpecifiedForAutoCtorSelection, request); ``` From the first glance, the method should throw if constructor isn't found. But this line returns null if the constructor doesn't satisfy the given condition: ```charp if (additionalToPublicAndInstance == 0) return condition == null || condition(ci.DeclaringType, ci.GetParameters()) ? new FactoryMethod(ci) : null; ``` Not sure but maybe it should also throw a Constructor not found exception?

UPD: nevermind, I re-read the exception message, and it says: implementation type is not specified, not constructor not found. — My bad!

dadhi commented 7 months ago

@yallie Hi, looks clean! Merging.

yallie commented 7 months ago

Thanks! 👍