Skip to content
This repository was archived by the owner on Nov 2, 2018. It is now read-only.

It's not ISSUE but Question #305

Closed
comdiv opened this issue Oct 15, 2015 · 8 comments
Closed

It's not ISSUE but Question #305

comdiv opened this issue Oct 15, 2015 · 8 comments
Labels

Comments

@comdiv
Copy link

comdiv commented Oct 15, 2015

We have own DI/IoC framework in private code. It has 5 years history and primary was inspired by Castle Windsor.
We think that it will be good if aspnet/DI became the only one to decrease entropy of different solutions. But our framework has greatly another functionality rather than current apnet/DI way. If it's interesting we will try supply improved version of DI if it's not.. it's not...

Here is some features we suggest to implement (pseudocode - not real names).
Most of them focused on adaption IoC as plugin centralization and extension point centralization facility.

  1. DI-bound services - service instance can implement plugin-like API void OnCreateFromContainer(IContainer container, IComponent component) - so service can realize own DI logic and use container internally

  2. Automatic assembly and type setup a) classes that implement IContainerLibrarySetup.Setup(IContainer container) - can be used as assembly-wide DI setup logic and[ContainerComponentAttribute(lifestyle,...)] that provide decoration for services that we want to register in container, at Container level there are Container.Register(Assembly assembly); and Container.Register(Type type) extension methods

  3. Support for multiple mapping ServiceType -> ServiceImpl* that allow as not just Resolve<S>() but ResolveAll<S>() notation - so it's simple way to inject collections of some services (it's usual for extensions, filters, subproviders and so on)

  4. Dual type-based and name-based registry for components, so we can use both Resolve<S>() or Resolve(Type t) and Resolve(name) Resolve<S>(name) and Resolve<S>("prefix*") - this is very usefull if we have multiple services for service interface but for different tasks. For example we have some kind ofIFilter( bool IsFiltered(object val) )but context of filter is very different. We can use namespace-like name notation and callResolveAll("out.type.*")``` to get them

  5. Extended [Inject] attribute that can decorate Properties, Fields and Method parameters - it can define some hints about how to bind it's target during DI when service is created and allow explicit control how to setup service. So it can provide required name (4) , default factory type (if not container setup) and so on

  6. DI not for public only - it can bind privates if it's explicitly required by Inject, DI can work with collections, DI can keep existed collections

In code it looks like:

[ContainerComponent(Lifiestyle.Singleton)]
class MyServive:IOurService {
      [Inject]
      protected IList<IOtherService> SubServices {get;}=new List<IOtherService>{new OtherService()};
      ...

}

var c = new Container();
c.Register<MyService>();
...
c.Resolve<IOurService>(); // internally SubServices will be appended with DI with keeping existed list
...
  1. Inject API that allow us DI into existed classes, aside of main DI process (Angular DI inspired):
IContainer c = GetMyMainAppContaner(); // it's not really-existed method )))
var myClass = new MyObject();
c.Inject(myClass);

This is things that we think to realize in aspnet/DI.

If it's interesting - we will fork DI, create set of focused issues and start to setup contribute.

@pranavkm
Copy link
Contributor

In general, we've been avoiding adding features that don't exist in other DI solutions.
That said, some of these could be solved with what we have in the box.

  1. You could use service factories to achieve something along these lines.
  2. Auto discovery can be built as an extension on top of the framework. Is there value in having it be built in?
  3. We support registering multiple services for the same type. ResolveAll<S> is writen as Resolve<IEnumerable<S>>.
  4. See Add an ability to bind same interface to several implementations #228 for discussion on named services.
  5. [Inject] is a feature of Mvc. I'm not super sure where we stand on this. Also, how would this look like and work with other service containers?
  6. See 5
  7. Isn't this equivalent to ActivatorUtilities.CreateInstance<MyObject>(serviceProvider)?

@khellang
Copy link
Contributor

I thought the whole point of this was to have a "lowest common denominator" way of registering/resolving framework components and that you could BYOC (Bring Your Own Container) if you wanted more features?

Several of the things mentioned are generally viewed as anti-patterns, like property injection (either using an InjectAttribute or an Inject method) or activating instances using a non-public constructor. It's already bad enough that this library encourages service location.

Some of the requirements of this library (see conforming containers) are already alienating several existing .NET IoC containers and they're unable to conform. Adding more features to this list of requirements won't help anyone.

So, my 👍 goes to BYOC if you require more features from your IoC container than what this library offers. The current set of features should be more than enough to get by and somewhat enforces good practices like constructor injection 😄

@comdiv
Copy link
Author

comdiv commented Oct 15, 2015

Ok. BYOC is way. Property injection can be worst than evil but it's thing that any DI should provide along with ctor injection. So private members binding too if explicitly configured. But it's kind of holywar.

@Eilon Eilon added this to the Backlog milestone Jan 23, 2016
@Eilon
Copy link
Contributor

Eilon commented Jan 23, 2016

Moving this to Backlog as we are not planning to make any changes for this at this time.

@IhnatKlimchuk
Copy link

After implementing several projects in asp.net core faced with problem that ConfigureServices function contained spaghetti code of services configuration. Another problem was that information about ServiceLifetime etc. is connected with service itself and code cohesion wasn't perfect.

As solution created temporary library DIAttributeRegistrar where in readme named several problems. I agree that all new functionality mentioned in discussion above it's not required as part of Microsoft.Extensions.DependencyInjection package, but you can add new package with all sugar functionality for end-product developers.

@davidfowl
Copy link
Member

Nice! The biggest downside is loading all assemblies and scanning all types up front. That will dramatically slow down the startup time (especially as the application grows). You can make this a little smarter https://github.com/IhnatKlimchuk/DIAttributeRegistrar/blob/98b8a3641faf685f64417366b6344ee331bb52c6/src/DIAttributeRegistrar/DIAttributeRegistrarExtentions.cs#L39 by looking at assemblies that depend on the DIAttributeRegistrar package. That way you're not scanning everything by default.

@IhnatKlimchuk
Copy link

IhnatKlimchuk commented Jun 14, 2017

Thanks @davidfowl for pointing that, will add improvements shortly. Anyway you can pass assemblies to scan by your own.

Regarding this discussion, I hope to see new package soon. That definitely should be separate package in order not to decrease base performance. Will be glad to help with it.

@aspnet-hello
Copy link

This issue was moved to dotnet/aspnetcore#2352

@aspnet aspnet locked and limited conversation to collaborators Jan 1, 2018
@aspnet-hello aspnet-hello removed this from the Backlog milestone Jan 1, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

7 participants