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

Add support for convention based binding #24

Closed
svermeulen opened this issue Oct 28, 2014 · 11 comments
Closed

Add support for convention based binding #24

svermeulen opened this issue Oct 28, 2014 · 11 comments

Comments

@svermeulen
Copy link

In some cases it can be tedious to, for example, add every binding to ITickable or IInitializable explicitly in the installer. What would be useful is if you could define generic conventions to always handle cases like this. We could use another fluent interface, something like the following (which is largely taken from what other DI frameworks look like)

Container.Bind(x => x.AllAssemblies.AllClasses().WhichInheritFrom<ITickable>())
    .InterfaceToSingle()); 
@janus007
Copy link

It is an very important feature to have in a DI-framework which was one of the reasons I immediately felt in love with Structuremap 10 years ago :) , Autofac and other major DI-frameworks has it as well now.

The reason for scanning and/ or convention-based binding is that polymorphism is working flawlessly making it even more powerful to utilize a DI-framework.

One more thing to mention regarding binding, is the fantastic feature called open generics, this is where the world of magic opens up and you wonder if you ever will wake up from the dream.

@svermeulen
Copy link
Author

Yep, this is supported in Zenject 4, along with lots of other improvements, which is just a few days away from being ready to use.

As for open generics, I'm assuming you're talking about doing stuff like this?

Container.Bind(typeof(List<>)).AsSingle();

var stringList = Container.Resolve<List<string>>();

This has been supported for a long time but I haven't personally used it much. Maybe there are some interesting design choices with it that I'm missing

By the way, if you are already familiar with StructureMap, AutoFac, etc. I would love to hear more feedback :) Before working in Unity3D I only really used Ninject so I don't have much else to compare it to

@janus007
Copy link

janus007 commented Sep 20, 2016

Sorry, for the delay. I somehow missed the reply.
Anyway... you're right on target with:
Container.Bind(typeof(List<>)).AsSingle();

For readers: suppose we have different implementations of EnemyBrain:

interface IEnemyBrain<T>
void Think(T go);

and:

class EnemyBrain: IEnemyBrain<Clever>
void Think(Clever clever)
{
   clever.ExtendedPositionAwareness(....
}

class EnemyBrain: IEnemyBrain<Idiot>
void Think(Idiot idiot)
{
   idiot.SimpleStupidPositionAwareness(....
}

We could then do something like:

class Enemy(IEnemyBrain<Clever> brain)
{
void Update()
   brain.Think();
}

class Enemy(IEnemyBrain<Idiot> brain)
{
void Update()
   brain.Think()
}

Seems a bit tedious at first, but the smart thing is that all the wiring is done automatically and follow separation of concerns nicely.

@janus007
Copy link

janus007 commented Sep 21, 2016

Hello svermeulen

Just tested this with generics but couldn't get it to work. Validation failed.

public interface IFoo<T>
        {
            string Bar(T t);
        }

        public class FooString:IFoo<string>
        {
            public string Bar(string s)
            {
                return s;
            }
        }
        public class FooInt : IFoo<int>
        {
            public string Bar(int s)
            {
                return s.ToString();
            }
        }

Install Bindings:

   Container.Bind(typeof(IFoo<>)).AsSingle();

   var expectedInstanceOfFooString = Container.Resolve<FooString>();
   var expectedInstanceOfFooString = Container.Resolve<IFoo<string>>();
   var expectedInstanceOfFooInt = Container.Resolve<IFoo<int>>();

Cannot resolve any of them. Maybe the binding parameters are wrong?

Not sure if this can help, but normally I do like this in StructureMap:http://structuremap.github.io/generics/ , pay attention to : x.ConnectImplementationsToTypesClosing(typeof(IVisualizer<>));

Of course I can do the mapping by hand, but it would be really nice to have support for open generics scanning.

@svermeulen
Copy link
Author

Ok I looked into this a bit.

This binding:

Container.Bind(typeof(IFoo<>)).AsSingle();

Doesn't make any sense given the way the Zenject interprets it. That means that Zenject should instantiate an instance of the interface IFoo<> which is impossible.

In Zenject you have to explicitly declare every mapping from the "contract type" to the "concrete instantiated type". So I think you're trying to do this:

var container = new DiContainer();

container.Bind(typeof(IFoo<>)).To(x => x.AllTypes().DerivingFrom(typeof(IFoo<>))).AsSingle();
container.Bind(x => x.AllTypes().DerivingFrom(typeof(IFoo<>))).AsSingle();

var expectedInstanceOfFooString = container.Resolve<FooString>();
var expectedInstanceOfFooString2 = container.Resolve<IFoo<string>>();
var expectedInstanceOfFooInt = container.Resolve<IFoo<int>>();

This code almost works, except that the DerivingFrom method does not handle open generic types properly. I'll leave this item open to address that.

@janus007
Copy link

Excellent...
You are very helpful :)

@svermeulen
Copy link
Author

svermeulen commented Sep 22, 2016

Actually you could do it this way too:

var container = new DiContainer();

container.Bind(typeof(IFoo<>)).To(x => x.AllNonAbstractClasses()).AsSingle();
container.Bind(x => x.AllTypes().DerivingFrom(typeof(IFoo<>))).AsSingle();

var runner = container.Instantiate<Runner>();
var expectedInstanceOfFooString = container.Resolve<FooString>();
var expectedInstanceOfFooString2 = container.Resolve<IFoo<string>>();
var expectedInstanceOfFooInt = container.Resolve<IFoo<int>>();

When doing convention based binding, Zenject will throw away any bindings that do not derive from the contract type, so you can just bind to AllNonAbstractClasses instead of the specific derived types

But again, there's an issue with open types not being handled correctly when used with interfaces somewhere that needs to be fixed first

@svermeulen svermeulen removed this from the v4.0 milestone Feb 10, 2017
@pbastia
Copy link

pbastia commented Jul 11, 2017

Hi,
I ended up writing this method, because as @svermeulen mentioned, open types are not being handled quite correctly.
it works the same as x.ConnectImplementationsToTypesClosing() in StructureMap.

Not sure if this is something that could be used inside the Zenject code, but it works for my purpose:

public static void BindAllOpenGenericInterfacesToConcreteImplementation(this DiContainer container, Type TGenericInterface, string targetNamespace = "")
{
	// AppDomain.CurrentDomain.GetAssemblies().SelectMany(s => s.GetTypes())
	// Might be safer, we are not sure what is the calling assembly.
	var types = Assembly.GetExecutingAssembly().GetTypes();
			
      //WARNING without the distinct clause we end up binding twice
	var implementedInterfaces = types.SelectMany(type => type.GetInterfaces()).Where(iface => 
                iface.IsGenericType && iface.GetGenericTypeDefinition() == TGenericInterface).Distinct();
	var targetTypes = string.IsNullOrEmpty(targetNamespace) ? types : types.Where(t => t.Namespace == targetNamespace);
			
	foreach (var implementedInterface in implementedInterfaces)
	{
		var typesImplementing = targetTypes.Where(t => implementedInterface.IsAssignableFrom(t) && !t.IsInterface);
        
		container.Bind(implementedInterface).To(typesImplementing).AsSingle();
	}
}

I use it as follow:

this.Container.BindAllOpenGenericInterfacesToConcreteImplementation(typeof(INotificationHandler<>));

@svermeulen
Copy link
Author

I committed a fix just now that allows things like this to work:

Container.Bind(typeof(IFoo<>)).To(typeof(Foo<>)).AsSingle()

@Vorlex
Copy link

Vorlex commented Apr 11, 2018

Hi!
I've tested with this bindings:

public class MainInstaller : MonoInstaller<MainInstaller>
{
    public override void InstallBindings()
    {
        Container.Bind<IA>().To<A>().AsTransient();
        Container.Bind<IB>().To<B>().AsTransient();

        Container.Bind(typeof(IFoo<>)).To(typeof(Foo<>)).AsSingle();
        Container.Bind<Tester>().ToSelf().AsSingle().NonLazy();
    }

    public class Tester
    {
        public Tester(IFoo<IA> a, IFoo<IB> b){}
    }

    public class A : IA {}
    public class B : IB {}
    public interface IB {}
    public interface IA{}
    public class Foo<T> : IFoo<T>
    {
        public Foo()
        {
            Debug.LogWarning(typeof(T));
        }
    }
    public interface IFoo<T>{}
}

and it throws this exception on resolving IFoo<IB>:

ArgumentException: failed to convert parameters
System.Reflection.MonoCMethod.Invoke (System.Object obj, BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) (at /Users/builduser/buildslave/mono/build/mcs/class/corlib/System.Reflection/MonoMethod.cs:484)
System.Reflection.MonoCMethod.Invoke (BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) (at /Users/builduser/buildslave/mono/build/mcs/class/corlib/System.Reflection/MonoMethod.cs:528)
System.Reflection.ConstructorInfo.Invoke (System.Object[] parameters) (at /Users/builduser/buildslave/mono/build/mcs/class/corlib/System.Reflection/ConstructorInfo.cs:77)
Zenject.DiContainer.InstantiateInternal (System.Type concreteType, Boolean autoInject, Zenject.InjectArgs args) (at Assets/Plugins/Zenject/Source/Main/DiContainer.cs:962)
Rethrow as ZenjectException: Error occurred while instantiating object with type 'MainInstaller+Tester'
Zenject.DiContainer.InstantiateInternal (System.Type concreteType, Boolean autoInject, Zenject.InjectArgs args) (at Assets/Plugins/Zenject/Source/Main/DiContainer.cs:967)
Zenject.DiContainer.InstantiateExplicit (System.Type concreteType, Boolean autoInject, Zenject.InjectArgs args) (at Assets/Plugins/Zenject/Source/Main/DiContainer.cs:2477)
Zenject.TransientProvider+<GetAllInstancesWithInjectSplit>c__Iterator0.MoveNext () (at Assets/Plugins/Zenject/Source/Providers/TransientProvider.cs:62)
Zenject.CachedProvider+<GetAllInstancesWithInjectSplit>c__Iterator0.MoveNext () (at Assets/Plugins/Zenject/Source/Providers/CachedProvider.cs:50)
Zenject.IProviderExtensions.GetAllInstances (IProvider creator, Zenject.InjectContext context, System.Collections.Generic.List`1 args) (at Assets/Plugins/Zenject/Source/Providers/IProviderExtensions.cs:29)
Zenject.IProviderExtensions.GetAllInstances (IProvider creator, Zenject.InjectContext context) (at Assets/Plugins/Zenject/Source/Providers/IProviderExtensions.cs:18)
Zenject.DiContainer.SafeGetInstances (Zenject.ProviderPair providerPair, Zenject.InjectContext context) (at Assets/Plugins/Zenject/Source/Main/DiContainer.cs:819)
Zenject.DiContainer.ResolveDependencyRoots () (at Assets/Plugins/Zenject/Source/Main/DiContainer.cs:241)
Zenject.SceneContext.Resolve () (at Assets/Plugins/Zenject/Source/Install/Contexts/SceneContext.cs:268)
Zenject.SceneContext.RunInternal () (at Assets/Plugins/Zenject/Source/Install/Contexts/SceneContext.cs:137)
Zenject.RunnableContext.Run () (at Assets/Plugins/Zenject/Source/Install/Contexts/RunnableContext.cs:36)
Zenject.RunnableContext.Initialize () (at Assets/Plugins/Zenject/Source/Install/Contexts/RunnableContext.cs:22)
Zenject.SceneContext.Awake () (at Assets/Plugins/Zenject/Source/Install/Contexts/SceneContext.cs:113)

So I guess it still doesn't work.

@svermeulen
Copy link
Author

@Vorlex Your example runs now on develop branch, thanks for the report

As far as I know the issues with open generics are fixed now so closing this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants