Skip to content
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

No warning/error when constructor selection policy produces an ambiguous match #314

Closed
alexmg opened this issue Jan 22, 2014 · 5 comments
Closed
Labels

Comments

@alexmg
Copy link
Member

alexmg commented Jan 22, 2014

From travis.illig on April 01, 2011 09:13:44

If a registered type has multiple constructors, one of which is an IEnumerable, that constructor will be selected over a more specific constructor even if named parameters are specified.

I would expect a service resolution to pick the most specific constructor based on the available arguments. That doesn't seem to happen if one of the constructors has an IEnumerable argument; in that case, the IEnumerable is selected and set to empty rather than selection of the most specific constructor.

The following NUnit fixture demonstrates the issue:

[TestFixture]
public class DemoResolutionFixture
{
[Test]
public void Visit_ConstantValue()
{
var autofacParam = new NamedParameter("input", "abc");
var builder = new ContainerBuilder();
builder.RegisterType().WithParameter(autofacParam);
var container = builder.Build();
var resolved = container.Resolve();
// THIS FAILS:
Assert.AreEqual("abc", resolved.CtorParameter);
}

private interface ISampleService
{
}

private class RegisteredComponent
{
public object CtorParameter { get; private set; }
public RegisteredComponent(ISampleService service)
{
Console.WriteLine("Service CTOR");
this.CtorParameter = service;
}
public RegisteredComponent(IEnumerable services)
{
// THIS CTOR WILL BE SELECTED
// EVEN THOUGH A STRING PARAMETER WAS PROVIDED.
// COMMENT THIS CTOR OUT AND THE TEST PASSES.
Console.WriteLine("IEnumerable CTOR");
this.CtorParameter = services;
}
public RegisteredComponent(string input)
{
// THIS CTOR SHOULD BE SELECTED BUT ISN'T.
Console.WriteLine("String CTOR");
this.CtorParameter = input;
}
}
}

Original issue: http://code.google.com/p/autofac/issues/detail?id=314

@alexmg
Copy link
Member Author

alexmg commented Jan 22, 2014

From travis.illig on March 31, 2011 16:14:39

Using Autofac 2.4.5.724 with .NET 4.

@alexmg
Copy link
Member Author

alexmg commented Jan 22, 2014

From nicholas...@gmail.com on April 04, 2011 03:20:32

Interesting - might need to give this one some thought.

Autofac currently doesn't distinguish between supplied and resolved parameters, so it really just sees that two constructors can be satisfied and rolls a dice to pick one.

Given the parameter architecture I don't think this could be changed very easily, but I see how behaviour in this situation is tricky. Perhaps we should just throw when multiple constructors with the same number of parameters match? The extra line of configuration with UsingConstructor(x) would be a small pain, but the unpredictable choice is probably worse...

@alexmg
Copy link
Member Author

alexmg commented Jan 22, 2014

From travis.illig on April 04, 2011 07:56:00

I must just be getting lucky then, or maybe I'm not running into this as often as I think I am. I always just assumed the behavior was something like:

  • Get all the possible constructors.
  • Get all the container-supplied arguments.
  • Get all the manually-supplied parameters.
  • From all the constructors, see which one matches the most manually-supplied parameters.
  • Failing that, fall back from most-available-parameters to least-available-parameters. (That is, "most specific to least specific.")

The need to use UsingConstructor wasn't clear from http://code.google.com/p/autofac/wiki/Autowiring - it looked like that was only necessary to override the default behavior of "most to least specific constructor." My misunderstanding.

In the short term, I'd propose two things:

  1. Enhance the docs to explain the logic of which constructor is chosen, specifically when there's a "tie" and one gets randomly chosen.
  2. Throw an exception when there's an ambiguous constructor selection to force folks to use UsingConstructor to choose.

Longer term, it'd be nice to have behavior like I described - where manually supplied parameters take precedence over container parameters and influence the logic that infers which constructor to select. I'm guessing folks wouldn't go to the trouble of manually specifying parameters if they didn't intend for them to be used.

@alexmg
Copy link
Member Author

alexmg commented Jan 22, 2014

From nicholas...@gmail.com on April 09, 2011 23:16:17

Sounds good. I'm looking at #2 now - in the process optimising a bit of the hot path through constructor selection, so worth spending some time in there anyway.

Summary: No warning/error when constructor selection policy produces an ambiguous match
Status: Started

@alexmg
Copy link
Member Author

alexmg commented Jan 22, 2014

From nicholas...@gmail.com on April 10, 2011 03:34:15

Exception now thrown in the case of ambiguity.

Status: Fixed

@alexmg alexmg closed this as completed Jan 22, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant