From df0d28b57e51264b1bb2fdfe5e3211e77523f206 Mon Sep 17 00:00:00 2001 From: Matthias Vill Date: Wed, 28 Nov 2018 07:43:09 +0100 Subject: [PATCH] Don't immediately throw in ConstructorInjectionConvention (#181) --- .../ConstructorInjectionConvention.cs | 62 ++++++++----------- .../ConstructorInjectionConventionTests.cs | 12 ++-- 2 files changed, 32 insertions(+), 42 deletions(-) diff --git a/src/CommandLineUtils/Conventions/ConstructorInjectionConvention.cs b/src/CommandLineUtils/Conventions/ConstructorInjectionConvention.cs index b8b46dec..d0c354af 100644 --- a/src/CommandLineUtils/Conventions/ConstructorInjectionConvention.cs +++ b/src/CommandLineUtils/Conventions/ConstructorInjectionConvention.cs @@ -57,46 +57,33 @@ private void ApplyImpl(ConventionContext context) .GetTypeInfo() .GetConstructors(BindingFlags.Public | BindingFlags.Instance); - if (constructors.Length == 0) - { - throw new InvalidOperationException(Strings.NoAnyPublicConstuctorFound(typeof(TModel))); - } - - var (matchedCtor, matchedArgs) = (constructors.Length == 1) - // shortcut for single constructor - ? FindMatchedConstructor(constructors, context.Application, - throwIfNoParameterTypeRegistered: true) - // find the constructor with the most parameters first - : FindMatchedConstructor(constructors, context.Application, - throwIfNoParameterTypeRegistered: false); + var factory = FindMatchedConstructor(constructors, context.Application, + constructors.Length == 1); - var parameterLessCtor = Array.Find(constructors, c => c.GetParameters().Length == 0); - - if (matchedCtor == null && parameterLessCtor != null) + if (factory != null) { - return; - } - - if (matchedCtor == null && parameterLessCtor == null) - { - throw new InvalidOperationException(Strings.NoMatchedConstructorFound(typeof(TModel))); - } - - if (matchedCtor != null) - { - (context.Application as CommandLineApplication).ModelFactory = - () => (TModel)matchedCtor.Invoke(matchedArgs); + ((CommandLineApplication) context.Application).ModelFactory = factory; } } - private (ConstructorInfo matchedCtor, object[] matchedArgs) FindMatchedConstructor( + private static Func FindMatchedConstructor( ConstructorInfo[] constructors, IServiceProvider services, bool throwIfNoParameterTypeRegistered = false) { + if (constructors.Length == 0) + { + return () => throw new InvalidOperationException(Strings.NoAnyPublicConstuctorFound(typeof(TModel))); + } + foreach (var ctorCandidate in constructors.OrderByDescending(c => c.GetParameters().Length)) { var parameters = ctorCandidate.GetParameters().ToArray(); + if (parameters.Length == 0) + { + return null; + } + var args = new object[parameters.Length]; for (var i = 0; i < parameters.Length; i++) { @@ -106,22 +93,25 @@ private void ApplyImpl(ConventionContext context) { if (!throwIfNoParameterTypeRegistered) { - continue; + goto nextCtor; } - throw new InvalidOperationException(Strings.NoParameterTypeRegistered(ctorCandidate.DeclaringType, paramType)); + + return () => + throw new InvalidOperationException( + Strings.NoParameterTypeRegistered(ctorCandidate.DeclaringType, paramType)); } + args[i] = service; if (i == parameters.Length - 1) { - var matchedArgsLength = args.Count(x => x != null); - if (parameters.Length == matchedArgsLength) - { - return (ctorCandidate, args); - } + return () => (TModel) ctorCandidate.Invoke(args); } } + + nextCtor: ; } - return (null, null); + + return () => throw new InvalidOperationException(Strings.NoMatchedConstructorFound(typeof(TModel))); } } } diff --git a/test/CommandLineUtils.Tests/ConstructorInjectionConventionTests.cs b/test/CommandLineUtils.Tests/ConstructorInjectionConventionTests.cs index 098aba8c..2c16cc79 100644 --- a/test/CommandLineUtils.Tests/ConstructorInjectionConventionTests.cs +++ b/test/CommandLineUtils.Tests/ConstructorInjectionConventionTests.cs @@ -130,9 +130,9 @@ private TestAppWithoutPublicConstructor() public void ThrowsWhenNoAnyPublicConstructorFound() { var app = new CommandLineApplication(); - var ex = Assert.Throws(() => app.Conventions.UseConstructorInjection()); - Assert.IsType(ex.InnerException); - Assert.Equal(Strings.NoAnyPublicConstuctorFound(typeof(TestAppWithoutPublicConstructor)), ex.InnerException.Message); + app.Conventions.UseConstructorInjection(); + var ex = Assert.Throws(() => app.Model); + Assert.Equal(Strings.NoAnyPublicConstuctorFound(typeof(TestAppWithoutPublicConstructor)), ex.Message); } private class TestAppWithoutMatchedConstructor @@ -149,9 +149,9 @@ public TestAppWithoutMatchedConstructor(TestConsole console) public void ThrowsWhenNoMatchedConstructorFound() { var app = new CommandLineApplication(); - var ex = Assert.Throws(() => app.Conventions.UseConstructorInjection()); - Assert.IsType(ex.InnerException); - Assert.Equal(Strings.NoParameterTypeRegistered(typeof(TestAppWithoutMatchedConstructor), typeof(TestConsole)), ex.InnerException.Message); + app.Conventions.UseConstructorInjection(); + var ex = Assert.Throws(() => app.ModelFactory()); + Assert.Equal(Strings.NoParameterTypeRegistered(typeof(TestAppWithoutMatchedConstructor), typeof(TestConsole)), ex.Message); } private class TestAppWithAlternativeConstructor