Constructor with IEnumerable<T> parameters selection

[Thedric Walker]

I discovered the following while trying to make a patch to submit a
ninject container adapter for FubuMVC.
Using the source commited to the ninject git repository on 08/27/2009
the following fails:

[Fact]
public void
ServiceIsInjectedWithArrayOfAllAvailableDependencies()
{
kernel.Bind<IParent>().To<RequestsArray>();
kernel.Bind<IChild>().To<ChildA>();
kernel.Bind<IChild>().To<ChildB>();

var parent = kernel.Get<RequestsArray>();

parent.ShouldNotBeNull();
parent.Children.ShouldNotBeNull(); //<--- Fails here.
parent.Children.Count.ShouldBe(2);
parent.Children[0].ShouldBeInstanceOf<ChildA>();
parent.Children[1].ShouldBeInstanceOf<ChildB>();
}

When RequestArray is defined as:

public class RequestsArray : IParent
{
public IList<IChild> Children { get; private set; }
public RequestsArray(){}
public RequestsArray(IChild[] children)
{
Children = children;
}
}

The reason that its fails could be related to this revision of ninject
no longer preferring the constructor with the most arguments by
default.

Well, after stepping through the test, I think I have found the
problem. Since the IEnumerable<T> parameter has a key in the Bindings
table with no values associated to it, the current code that uses the
number of Targets to choose the constructor does not recognize that
enumerable parameters as having targets already. When I added another
constructor ie a dummy one with no parameters, it and the one with the
IEnumerable<T> parameter had the same score. The selector thus
whichever is first in the directive sequence is the one that gets
picked to construct the service. So the reason why the test in
EnumerableDependenciesTests.cs still worked is because the classes
they are run against only have one constructor.

I guess that the real probelm is that the way things are now the
constructor scorer is giving false negatives on IEnumerable<T>
parameters.

[Ian Davis]

Do you have any suggestions for the .ctor scoring algorithm? The
algorithm was changed to try to select the best .ctor that is
satisfiable - whereas the previous algorithm would select .ctors which
could not be used.

-Ian

--
Ian Davis

[Ben Ellis]

Ian,

Would it be too auto-magic to make it so it scores the .ctor with
the most parameters that can be resolved.

E.g.

Public MyClass();
Public MyClass(string name);
Public MyClass(IMyDep myDependency);
Public MyClass(string name, IMyDep myDependency);

So in this case, if I've bound IMyDep to MyDepImple, but not bound
anything for a string parameter called "name". The third .ctor would be
called.

This could possibly throw an Exception if two .ctors have a matching
number of resolvable parameters.

i.e.

Public MyClass(IMyDep myDependency);
Public MyClass(IMyDep2 myDependency2);

That could be resolved by setting a preference,

i.e.

kernel.Bind<IMyDep>().To<MyDepImple>().PreferableTo<IMyDep2>();

Regards,

Ben

[Ian Davis]

::See Below

>        Would it be too auto-magic to make it so it scores the .ctor with
> the most parameters that can be resolved.

This is what it currently does. The scorer will add one point for each
parameter for which there exists a binding on the parameter type. If
you specify an inject attribute, it will give that .ctor the highest
score.

>        So in this case, if I've bound IMyDep to MyDepImple, but not bound
> anything for a string parameter called "name". The third .ctor would be
> called.

The third should be called as it stands right now (I believe) as it is
tied for the highest score and is last in the list and when sorted,
should be on the top of the list.

> kernel.Bind<IMyDep>().To<MyDepImple>().PreferableTo<IMyDep2>();

Not sure about this. You it seems like there is something wrong with
the design or intention if your object is set up in that fashion. Can
you give a real example?

Thanks,

Ian

--
Ian Davis

[Thedric Walker]

I've been trying to think of a way. The only idea I could think of
sort of smells. That is to add a special case similar to what is in
the Target.ResolveWithin() method.

[Ben Ellis]

Ian,

Reply inline below.

> ::See Below
>
> >        Would it be too auto-magic to make it so it scores the .ctor
> > with the most parameters that can be resolved.
>
> This is what it currently does. The scorer will add one point for each
parameter for which there exists a binding on the parameter type. If you
specify an inject attribute, > it will give that .ctor the highest score.

I apologise. The last build that I downloaded only did it on the .ctor with
the most parameters (whether they were resolvable or not). I hadn't realized
this had changed.

>
> >        So in this case, if I've bound IMyDep to MyDepImple, but not
> > bound anything for a string parameter called "name". The third .ctor
> > would be called.
>
> The third should be called as it stands right now (I believe) as it is
tied for the highest score and is last in the list and when sorted, should
be on the top of the list.
>
> > kernel.Bind<IMyDep>().To<MyDepImple>().PreferableTo<IMyDep2>();
>
> Not sure about this. You it seems like there is something wrong with the
design or intention if your object is set up in that fashion. Can you give a
real example?

I encountered this when I first starting using Ninject and wasn't
using it correctly. I was stupidly using it to resolve a data provider, from
the System.Data library. One of the arguments on the provider I was using
was an optional string argument, that I wasn't using anyway.

>
> Thanks,
>
> Ian

Thanks,

Ben

[Thedric Walker]

OK, What do you think? Here is how I reimplemented
StandardConstructorScorer.Score() so that the above test will pass:

public int Score(IContext context, ConstructorInjectionDirective
directive)
{
Ensure.ArgumentNotNull(context, "context");
Ensure.ArgumentNotNull(directive, "constructor");

if(directive.Constructor.HasAttribute(Settings.InjectAttribute))
return Int32.MaxValue;
int score = 1;
foreach(ITarget target in directive.Targets)
{
Type targetType = target.Type;

if (targetType.IsArray)
{
targetType = targetType.GetElementType();
}

if (targetType.IsGenericType && targetType.GetInterfaces().Any
(t=>t == typeof(IEnumerable)))
{
targetType = targetType.GetGenericArguments()[0];
}

if(context.Kernel.GetBindings
(targetType).Count() > 0)
{
score++;
}

}
return score;
}

What do you think? Did I miss something? Is there a better way?

[Ian Davis]

I don't see anything wrong with it. I added a few unit tests and
everything passes. I am going to sit on it a little and see if
anything else should be done.

Thanks!

Ian

--
Ian Davis

[Thedric Walker]

Cool. Glad I could help.

[Ian Davis]

I committed a fix that should resolve your issues. Sorry it took so
long to get committed, it got buried in my task list.

Thanks,

Ian Davis
http://disassembla.net