ipjohnson / Grace

Grace is a feature rich dependency injection container library
MIT License
336 stars 33 forks source link

Wrong constructor selection for Func<Param,Injected> #278

Open mgth opened 3 years ago

mgth commented 3 years ago

I expected constructor two to be used with parameter of type ClassB but grace injects a newly created ClassA in the first constructor instead.

    class ClassA
    { }

    class ClassB
    {
    }

    class InjectedClass
    {
        public ClassA ClassA;
        public ClassB ClassB;

        public InjectedClass(ClassA classA)
        {
            ClassA = classA;
        }

        public InjectedClass(ClassB classB)
        {
            ClassB = classB;
        }
    }

    public class ConstructorSelection
    {
        [Fact]
        public void Test()
        {
            var container = new DependencyInjectionContainer();

            var funcA = container.Locate<Func<ClassA, InjectedClass>>();
            var classA = container.Locate<ClassA>();
            var injectedA = funcA(classA);

            var funcB = container.Locate<Func<ClassB, InjectedClass>>();
            var classB = container.Locate<ClassB>();
            var injectedB = funcB(classB);

            Assert.Equal(classA,injectedA.ClassA);
            Assert.Equal(classB,injectedB.ClassB); //first constructor used, with newly created ClassA and classB not used.
        }
    }
ipjohnson commented 3 years ago

Hi @mgth

Grace makes some trade offs to achieve the performance it does. One of those trade off is that it's going to pick one constructor for an object and stick with it. For corner cases like this it's not going to work very well.

Let's take a step back though because constructor injection is supposed to be the contract specifying the dependencies an object depends on to be constructed. What does it say when there are two constructors with two different parameter types?

My general rule of thumb these days is to have one constructor. If you want/need multiple have one main constructor that take all parameters and the others call the main constructor. That said it's usually far easier and more descriptive to have one constructor/one way to construct an object.

mgth commented 3 years ago

I understand the point, I ended up with creating two inherited classes, one for each case. Not as elegant in my case but maybe more readable, so it's ok. Thank you for the time pasted.