Fix Resolve Issue with Multiple AutoRegister Calls #155
Conversation
| if (registrationPredicate != null && !registrationPredicate(type)) | ||
| { | ||
| ignoreChecks.Add(t => !registrationPredicate(t)); | ||
| if (ignoreChecks.Any(c => c(type))) | ||
| return true; | ||
| } | ||
|
|
||
| for (int i = 0; i < ignoreChecks.Count; i++) | ||
| { | ||
| if (ignoreChecks[i](type)) | ||
| return true; | ||
| } | ||
|
|
||
| return false; | ||
| return registrationPredicate != null && !registrationPredicate(type); |
There was a problem hiding this comment.
Do we know the context on why it was checking the registration Predicate first? Can you please request a review from the person who wrote this?
There was a problem hiding this comment.
I think the intention of @AlexanderMy was to early return if the predicate matches but then he forgot to remove the line that was adding the predicate to the (previous local) list.
| @@ -3582,7 +3582,7 @@ where localType.IsAssignableFrom(implementationType) | |||
| } | |||
|
|
|||
| // TODO - find a better way to remove "system" assemblies from the auto registration | |||
| private readonly List<Func<Assembly, bool>> ignoredAssemlies = new List<Func<Assembly, bool>>() | |||
| private static readonly IReadOnlyList<Func<Assembly, bool>> ignoredAssemlies = new List<Func<Assembly, bool>>() | |||
There was a problem hiding this comment.
I'm not sure how I feel about this being static. Would someone ever have more than one container instance?
There was a problem hiding this comment.
I added IReadOnlyList to avoid modifications of the list but then realized my IDE suggests static ;-)
The assembly ignore list will not change if you have multiple containers. It could be written as (static) if's inside IsIgnoredAssembly but the list seems to be more elegant. Since the list is identically for all containers and will never change it can be made static to reduce memory consumption.
|
Thanks for the pr! |
The static list of ignore-types must not be changed to prevent side effects when resolving types with predicates.
Fixes #154