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

Proposal: Dependency Injection / Service Locator as a Language Feature #1690

Closed
dklompmaker opened this issue Jul 3, 2018 · 62 comments
Closed

Comments

@dklompmaker
Copy link

dklompmaker commented Jul 3, 2018

DI / Service Locator as Language Feature

The use of dependency injection frameworks has increased over the years; and for good reason. DI frameworks help to enforce good design patterns & principles. We have an opportunity to make CSharp the first language with IOC / DI as a language feature.

Proposal Version: v0.1

Discussion

This proposal isn't perfect and would benefit from some deep discussion on the potential. I think this proposal can lead CSharp toward a language that supports good design practices as a feature.

Example Code

Before we dive into the details, let's take a high-level look at the implementation. In our example, we have a service and a consumer. Depending on the static / non-static binding, the objects are resolved on demand. Each object's dependencies are then injected via the container automatically.

public container AppContainer
{
    public static IHttpService as HttpService;
    public ISomeType as SomeType;
}

public class SomeType : ISomeType
{
    public IHttpService HttpService { get; set; }

    public SomeType(inject IHttpService httpService)
    {
         HttpService = httpService;
    }
}

public class Program
{
     public static void Main(string[] args)
     {
          var httpService = IHttpService from AppContainer;
          var someType = ISomeType from AppContainer;
          var someType2 = ISomeType from AppContainer;
         
         // ISomeType is bound as a transient, therefore unique instances...        
         Assert.NotEqual(someType, someType2);

         // IHttpService is bound statically, therefore acts like a singleton...
         Assert.AreEqual(httpServie, someType.HttpService);
     }
}

Details

Inject Modifier

An inject modifier would make the parameter act like an optional value. If not provided, it would take default(T). The compiler would generate the variations with and without the values passed in.

public class SomeType : ISomeType
{
    public SomeType(inject IHttpService httpService) { }
    public SomeType() {} // Generated...
}

var someType = new SomeType(service);
var someType2 = new SomeType(); // inject treated as optional...

There are cases where constructors cannot be used; For instance, in game engines like Unity3D. In these cases, we could support using the inject modifier on public properties or fields. Typically this is bad practice since it's harder to mock; however we can support these edge cases.

public class SomeOtherType : ISomeType
{
    public inject IHttpService HttpService { get; set; }
}

Container

Typical DI frameworks define a Container that acts like a service locator. The language would introduce a new entity type called container.

public container AppContainer
{
     public static IHttpService as HttpService;
     public virtual ISomeType as SomeType;
}

Container Inheritence

Inheriting other Containers would share the bindings between the two. Virtual bindings can be overridden just like properties or methods of a class.

public container OtherContainer : AppContainer
{
     public override ISomeType as OtherType;
}

Container Behavior Customization

You can customize the behavior of the Container by implementing a language provided interface.

public class MyResolveStrategy : IContainerResolveStrategy
{
    public TInstance Resolve<TInstance>(IResolveContext resolveContext) { ... }
    public object Resolve(IResolveContext resolveContext) { ... }
}

[ContainerResolveStrategy<MyResolveStrategy>()]
public container AppContainer
{
    ...
}

Binding

Binding types will be done through the container.

Binding Transient Types

public container AppContainer
{
    public ISomeType as SomeType;
}

Binding Singleton Types (Statics)

public container AppContainer
{
    public static IHttpService as HttpService;
}

Binding with custom factory

Sometimes you need to customize the creation of the types when you resolve. You can instead bind a factory method to the type. The parameters are evaluated and injected at the time of resolution.

public container AppContainer
{
    public static IHttpService(inject ISomeDependency someDependency) 
    {
        return new HttpService(someDependency);
    }
}

Binding types to the container at Runtime

This part needs work.

var instance = new SomeType();
AppContainer.Bind<ISomeType>().To(instance);

Runtime Resolving

As with traditional DI frameworks, the client can resolve a bound type. This would be supported in the language through syntactic sugar.

var someType = ISomeType from AppContainer;
var someType2 = ISomeType from OtherContainer;

Default Interface Constructors

Interfaces would have the ability to have default constructors that must be used by the implementing classes. The value of this can be seen in the next section.

public interface ISomeType
{
     ISomeType(inject IHttpService httpService);
}

Resolving with additional parameters

Since we now allow for default constructors on the interfaces, we can pass through additional (non-injected) parameters at the time of resolving.

public interface ILogger
{
     ILogger(string logPath);
}

public class FileLogger : ILogger
{
     public FileLogger(inject IFileWriter fileWriter, string logPath) : base.ILogger(logPath) { ... }
     public ILogger(string logPath) {  ... }
}

public container AppContainer
{
    public static ILogger as FileLogger;
}
... 

var logger = ILogger("some/path/log.txt") from AppContainer;

Unit Testing

Since the inject modifier is treated like an optional parameter, you still have the ability to pass in the dependencies through the constructor. Thus, mocking and unit-testing the classes is no different than how it is done today.

var someType = new SomeType(mockHttpService.Object);

Assert.AreEqual(...);
@CyrusNajmabadi
Copy link
Member

DI frameworks tend to have a lot of power and flexibility associated with them. It seems valuable that this sort of thing be present just in library so that the different frameworks can be appropriately opinionated, and can also make whatever changes/improvments they want at their own pace.

What substantive value do i get here? for example, i already do this today:

public SomeType([Import]IHttpService httpService)

Being able to say:

public SomeType(inject IHttpService httpService)

Doesn't seem substantively better. It also seems far weaker given all the flexibility i have to customize what something like [Import] does.

For example, do i want to fail if i can't import? Or just use null? What if i have an arbitrary collection type? how do i tell the language how to [ImportMany] it like i can with a DI framework? etc. etc.

@leonidumanskiy
Copy link

@CyrusNajmabadi

Doing [Import]IHttpService httpService or [Inject]IHttpService httpService is a bad practice.

By doing so you couple your code to a specific DI framework implementation and it makes it pretty much impossible to re-use (if I use Autofac and your library is using Microsoft Unity, I can't have both working together).

Same goes with using Container.Resolve - if you want to resolve types during the runtime, you either have to couple with a specific DI implementation, or you need to implement factory for each type you want to resolve if you want to keep your code clean.

Having standard, language built-in dependency injection solves both of these problems.

@scalablecory
Copy link

scalablecory commented Jul 3, 2018

Don't make me triple-type my injected fields (field, constructor parameter, constructor assignment) -- this is error-prone and can be extremely bothersome if you're injecting 10 different things. Using properties is not a good workaround: these injected members should usually not be part of your public interface.

Boiling it all down into something like this would be fantastic:

public class SomeOtherType
{
    inject readonly IHttpService httpService;
}

Perhaps it could generate a hidden constructor behind the scenes.

@jnm2
Copy link
Contributor

jnm2 commented Jul 3, 2018

var httpService = IHttpService from AppContainer;

This is service location, which many strongly consider an anti-pattern.
I'd go so far as to say that dependency injection is the precise opposite of service location. The RRR pattern is a good way to tell if you're using injection or location.

@jnm2
Copy link
Contributor

jnm2 commented Jul 3, 2018

@scalablecory I really want something like that. My only concern is scoping.

@dklompmaker dklompmaker changed the title Proposal: Dependency Injection as a Language Feature Proposal: Dependency Injection / Service Locator as a Language Feature Jul 3, 2018
@dklompmaker
Copy link
Author

@jnm2 You're absolutely correct. Using the service locator pattern at the composition root can kick-off the injection. Beyond the initial bound types, one can bind factories to then inject for later use.

public ISomeTypeFactory as SomeTypeFactory;

...

public class SomeOtherType
{
    public SomeOtherType(inject ISomeTypeFactory someTypeFactory)
    {
        var someType = someTypeFactory.Create(...);
    }
}

@dklompmaker
Copy link
Author

@scalablecory my only concern with removing the injection at the constructor level would be to enable mocking. But you are correct, if we can generate the constructors automatically, then one can still mock the injected types.

@HaloFour
Copy link
Contributor

HaloFour commented Jul 3, 2018

I'm with @CyrusNajmabadi, DI contains a massive amount of policy and configuration, sometimes too much. Trying to capture that in the language wouldn't be reasonable and frankly would only result in:

image

The wide ecosystem for containers (and the ability to not use containers at all) is a good thing.

@theunrepentantgeek
Copy link

I'm really conflicted by this proposal. On the one hand, dependency injection is really beneficial design pattern, one that could use a push to drive wider adoption. On the other hand, it's a very opinionated field where different existing libraries have very different approaches.

You also haven't touched on testing. It's really common to have one set of bindings/policy for production and a very different set for testing (often different for unit testing vs integration testing vs approval testing and so on).

A class that's a singleton in production often has many instantiations for testing purposes.

A concrete implementation (say, a logger) is often replaced with a fake (or a mock) when testing.

And so on.

@iam3yal
Copy link
Contributor

iam3yal commented Jul 3, 2018

@dklompmaker What's the benefit of adding all this complexity to the language when there are plenty of libraries/framework that do it today? why do we need to have a container and an inject method? keywords aren't free.

How will this work with existing libraries? should it "just work" ? how will this solve the problem for them? what problem does this solve for the consumer that can't be solved today?

I can understand the idea behind adding some primitive patterns into the language like the spoken builder pattern or any creational pattern to make it easier to create objects but adding something like this is a major undertaking and your examples are extremely simple, you barely touched the tip of the iceberg.

@dklompmaker
Copy link
Author

dklompmaker commented Jul 3, 2018

@theunrepentantgeek One shouldn't unit-test the container itself. You could test it, but it becomes an integration test at that point. The proposal lays out that the inject tags are treated as optional via the constructor generation. This means, that you can skip the service locator all together and simply pass in your dependencies. Whether you do so manually or through a framework like Moq.

// runtime usage...
var someService = ISomeService from AppContainer;

// unit test for some service...
var someService = new SomeService(dependency.Object);

Assert.AreEqual(...)

@dklompmaker
Copy link
Author

dklompmaker commented Jul 3, 2018

@eyalsk I appreciate the discussion. The idea behind this is that often you couple yourself to a specific DI framework when developing. There are very common patterns that are shared amongst all frameworks. If you could boil these down to language level syntactic sugar, then you could expose middleware or strategy pattern to allow for customization (if necessary).

The proposal provides a contract by which we can define common DI patterns. This contract then can be hot-swapped for any customized implementation.

@CyrusNajmabadi
Copy link
Member

CyrusNajmabadi commented Jul 3, 2018

The proposal provides a contract by which we can define common DI patterns. This contract then can be hot-swapped for any customized implementation.

Why can't that be done at a library level as well. Someone defines things like [Inject], and then writes bridge libraries that allow this to map onto Insert DI library de-jure that you like. You can 'hot swap' just fine here as long as there's an appropriate bridge to whatever specific DI system you want.

Why would that not work?

If you could boil these down to language level syntactic sugar

I mean... we have syntactic sugar for that exact purpose. They're attributes. They're very much the 'solution' when you want to be able to mark things in an nondescript manner and then have libraries take over the lifting of waht to do there...

then you could expose middleware or strategy pattern to allow for customization (if necessary).

This would greatly increase the size of this feature. Why not just have a library for this? It can define tehse special attributes, as well as whatever extensbility strategy it wants for customization all at once. No need for any language changes whatsoever...

@CyrusNajmabadi
Copy link
Member

Doing [Import]IHttpService httpService or ```[Inject]IHttpService httpService`` is a bad practice.

I don't understand this. Why is [Inject]IHttpService bad... but inject IHttpService not bad? However you intend inject IHttpService to work... why not just do exactly the same thing, except with [Inject]IHttpService. What do you expect the language would actually do here differently than what could just be done in a library...

i.e. define [MyNewCoolInjectAttribute] and do exactly what you want with it that you would have wanted the language to do with an inject keyword...

@dklompmaker
Copy link
Author

dklompmaker commented Jul 3, 2018

@CyrusNajmabadi Thanks for the discussion.

Why can't that be done at a library level as well.

This is the obvious path. Of course, we have many IOC / DI libraries that provide these type of features. However, taken the recent example of the addition of async to the language. It wasn't needed. There were plenty of patterns and libraries to add support for doing asynchronous calls. It was in the hopes of simplifying the process to create more elegant and cleaner code that it was proposed.

Why is [Inject]IHttpService bad.

It couples you to a specific DI framework. It's more than just the simple attribute, it's the other bootstrap code that becomes verbose. The goal of the proposal is to find common patterns in DI frameworks and see what it could look like as a language feature.

This would greatly increase the size of this feature. Why not just have a library for this? It can define tehse special attributes, as well as whatever extensbility strategy it wants for customization all at once.

In the same way that LINQ has brought a cleaner way to query, order and select results all within the realms of syntactic sugar, this proposal would seek to beautify and simplify the DI process. Sure, one could write a LINQ library that does what it does, but it's the addition for the language support that makes it beautiful.

@HaloFour
Copy link
Contributor

HaloFour commented Jul 3, 2018

@dklompmaker

Having a special inject syntax is just as bad as having an attribute from some arbitrary IoC container library, it still binds you to some implementation with all of its opinions, even if that implementation is just attempting to be some kind of facade.

The only pattern necessary for dependency injection is constructor parameters. There's no reason to adorn your components with anything else.

@jnm2
Copy link
Contributor

jnm2 commented Jul 3, 2018

@dklompmaker

The idea behind this is that often you couple yourself to a specific DI framework when developing.

If you do it properly, using factories instead of service location, the only dependency on the DI framework is in Program.Main. I'm playing around with an idea for an analyzer that generates all the code to set you up with Poor Man's DI at compile time instead of resolving all the things at runtime.

What I don't like about DI is the boilerplate, and the pain when refactoring when more and more constructor parameters have crept in. Worst, when inheritance or manual composition are involved. I started a discussion on this a while back but it didn't go anywhere: dotnet/roslyn#16745

@iam3yal
Copy link
Contributor

iam3yal commented Jul 3, 2018

@dklompmaker

The idea behind this is that often you couple yourself to a specific DI framework when developing.

Are you? what do you think about this? isn't this too late? do you know how many efforts went into adopting this abstraction and ,NET Core in general? it was immense, so now you're telling these authors to do it again but this time it should be specific to C#.

@dklompmaker
Copy link
Author

@HaloFour

Having a special inject syntax is just as bad as having an attribute from some arbitrary IoC container library

Is it? If it's part of the language feature-set, then you're bound to C#. It would be no different than coupling yourself by using async or LINQ.

@jnm2 I completely agree with you. The coupling is seen where frameworks provide attributes to control certain flows. This is where code becomes coupled with the framework.

@eyalsk

Are you? what do you think about this? isn't this too late? do you know how many efforts went into adopting this abstraction? it was immense, so now you're telling these authors to do it again.

I would argue that the fact this abstraction exist is evidence enough for a need. There clearly is a need for an abstracted layer for DI / Containers, that if done correctly at a language level, would provide huge benefits.

I make no assertions that it was easy. However, the sunk-cost fallacy is not a good argument against doing this.

@iam3yal
Copy link
Contributor

iam3yal commented Jul 3, 2018

@dklompmaker

I would argue that the fact this abstraction exist is evidence enough for a need.

It's done, it exists, people use it why we need another one that is C# specific? more importantly why this abstraction need to exist at the language level?

@CyrusNajmabadi
Copy link
Member

CyrusNajmabadi commented Jul 3, 2018

In the same way that LINQ has brought a cleaner way to query, order and select results all within the realms of syntactic sugar, this proposal would seek to beautify and simplify the DI process. Sure, one could write a LINQ library that does what it does, but it's the addition for the language support that makes it beautiful.

That's not true. You can't use attributes inside a method. And linq very much is about the actual expressions you end up writing. This is unlike this proposal, which seems to be entirely about annotating your external shape.

The same applies to your critique of 'async'.

But none of that seems to apply to DI.

@CyrusNajmabadi
Copy link
Member

It couples you to a specific DI framework. It's more than just the simple attribute, it's the other bootstrap code that becomes verbose. The goal of the proposal is to find common patterns in DI frameworks and see what it could look like as a language feature.

As i said [Inject] would be the meta pattern. it would simply do whatever it is you want inject to do. You could still use any DI framework you wanted. It's just the metaframework would allow you to swap them in. Basically, if you can see a way for inject to do it, then it seems like that could certainly be done just with [Inject].

If tehre is something that inject could do that [Inject] could not, could you specify what that would be?

@CyrusNajmabadi
Copy link
Member

in other words: What would you envision the C# compiler actually doing with this syntax? What does it actually emit into IL/metadata?

That part really needs to be explained.

@HaloFour
Copy link
Contributor

HaloFour commented Jul 3, 2018

@dklompmaker

DI doesn't share the same kind of shape as async or LINQ. The component isn't invoking some API to get its dependencies, it is (or should be) totally unaware of how that happens. The only interesting code happens on the outside, with the container, if you're using a container, which DI does not require. And that's where there is metric tons of policy and opinion that don't belong in a programming language.

@CyrusNajmabadi
Copy link
Member

I would argue that the fact this abstraction exist is evidence enough for a need.

I don't understand this argument at all :) Basically, using this logic literally every library out there should not exist (after all, it's an abstraction), and it should just become part of the language :D

@CyrusNajmabadi
Copy link
Member

There clearly is a need for an abstracted layer for DI / Containers

Let's say i agree with that. That there is some good case for an additional abstraction above what exists today (esp. to support this case of being able to swap things in/out). I don't see how it logically follows that that necessitates changes to the language itself.

It's totally possible to build another library which is "the abstraction layer that allows you to hotswap DI systems". Why is that not the right solutoin? Why is the right solution "language level" versus "library level"?

@CyrusNajmabadi
Copy link
Member

Note: i'm not trying to beat up on you rhere. I'm simply asking for a strong justification to be made for why this is appropriate at the language level. I can do that with many other features. But i'm not seeing an appropriate case being made for this suggestion.

@dklompmaker
Copy link
Author

dklompmaker commented Jul 3, 2018

@HaloFour & @CyrusNajmabadi I appreciate the discourse. I do understand the argument of: "Why can't this just be a library?". I think it's worth the thought experiment of what it could be like. For instance, it may be worth the effort to discuss whether promises or futures should be a language feature. But one could argue equally that it should be a library.

In fact, if you look at async + Task<> this is what we have today. We have a partial language support feature + base library. Sure, I can extend Task and customize it's behavior, but there is an inherit support for it out of the box.

The proposal lays out a similar paradigm that we have a language support + base library with potential for customization.

@DanFTRX
Copy link

DanFTRX commented Jul 3, 2018

@dklompmaker You seemed to miss the question of what "inject" can do that an attribute "[Inject]" cannot.

@ploeh
Copy link

ploeh commented Jul 3, 2018

C# doesn't need support for Dependency Injection (DI). That support has always been there, since version 1.0.

DI and Service Locator are opposites. DI is a set of patterns and principles (mostly, the SOLID principles); Service Locator, on the other hand, is an anti-pattern.

I once (2011) wrote a book about Dependency Injection in .NET. It was popular enough that a second edition is due out sometime this year. In it, I've explained all of this, and much more, in great detail. I'll refer the interested reader to the book for details, but the bottom line is that C# doesn't need any new features for DI support.

@HaloFour
Copy link
Contributor

HaloFour commented Jul 3, 2018

@leonidumanskiy

Interface constructors and optional injection might become a very powerful tool that gives you something that can not be done using attributes on a library level.

Interface constructors don't make sense for DI. They assume the dependencies that an implementation of that contract would require, but that's not a responsibility of the contract. I could have several different implementations, each with their own different dependencies, all bound and injected based on the some other context.

Optional injection is generally something that can be managed through a factory pattern, such as injecting an IFactory<IDependency> or Func<Dependency>. The container (or lack thereof) would not attempt to resolve the dependency until the component invoked that factory. It's service-locator and fragile but it does come in handy.

@HaloFour
Copy link
Contributor

HaloFour commented Jul 3, 2018

@dklompmaker

To me, the question of doing inject vs. [Inject] is equivalent between saying:

You missed my point in saying that you shouldn't be doing either. The component should have zero clue about dependency injection. You can't get less verbose than nothing at all.

@leonidumanskiy
Copy link

leonidumanskiy commented Jul 3, 2018

@HaloFour

Reiterating on my previous example, do you think it would make more sense if interface defined a constructor without dependencies i.e.:

public interface IApiService
{
    IApiService(string apiUrl);
}

Or, perhaps even this:

public interface IApiService
{
    IApiService();
}
...
IApiService service = new IApiService(); // dependencies are injected
service.SetUrl("http://");

This way you can resolve your service with all the dependencies without need to create factory that doesn't really do anything.

@HaloFour
Copy link
Contributor

HaloFour commented Jul 3, 2018

@leonidumanskiy

Neither. You don't construct IApiService, you have it handed to you.

@dklompmaker
Copy link
Author

dklompmaker commented Jul 3, 2018

@HaloFour

I disagree that interface constructors wouldn't make sense with DI. Take the following example:

public interface IApiService
{
    IApiService(IApiConfig configuration);
}

The constructor defines that any ApiService implementation, must take in an IApiConfiguration.

You could argue that instead of a constructor, one should do:

public interface IApiService
{
    void Setup(IApiConfig configuration);
}

The difference is, we're forcing a contract that we treat IApiConfig as a dependency rather than an additional object that is assigned. Thus, enforcing any implementation of IApiService treats IApiConfig as a constructor dependency.

@HaloFour
Copy link
Contributor

HaloFour commented Jul 3, 2018

@dklompmaker

The constructor defines that any ApiService implementation, must take in an IApiConfiguration.

Why? You don't know what any given implementation of IApiService does internally, as long as it fulfills its contract. And you shouldn't care.

Interfaces lack constructors on purpose; the creation of an implementation is not a part of their contract. That's true regardless of DI.

@leonidumanskiy
Copy link

leonidumanskiy commented Jul 3, 2018

@HaloFour

You don't construct IApiService, you have it handed to you.

How is it handled to you? Can you provide more real-world examples?
In my initial comment I provided an example of a "classical" approach to this problem which is to create a factory.

Factory approach works fine when you have a flat application like Rest Api using ASP.NET Core and you need to resolve and instantiate one or two things. However, if you try to develop i.e. a game server using this approach, this becomes a nightmare.

If you have your 1. IGameServer that creates 2. IRoom that creates 3. IMonster that casts 4. ISpell that creates 5. ICharacterStatus and you need to resolve all these things in the runtime and instantiate, you end up with IGameServerFactory, IRoomFactory, IMonsterFactory, ISpellFactory, ICharacterStatusFactory etc. And if you have 10 implementations of IMonster, you end up writing 10 factories.

You end up writing code that doesn't do anything, factories that you have to write simply exist to inject something that should be automatically injected because you can't use Resolve if you don't want to couple to a DI framework.

Delegate methods work in simple scenarios but not when you need to add something to your constructor.

@iam3yal
Copy link
Contributor

iam3yal commented Jul 3, 2018

@dklompmaker So not only you're asking to add DI support to the language but you want to add yet another feature to extend interfaces to support it? in this case, you might want to create a new issue asking for that and use-cases to support it independently from this proposal.

@dklompmaker
Copy link
Author

@eyalsk I appreciate the discussion, but the proposal contains this information. I want to have good discussion around this, but it's difficult if you don't read the proposal in full. It does include a section on default constructors for interfaces. Which is no different than this issue: #288

@iam3yal
Copy link
Contributor

iam3yal commented Jul 3, 2018

@dklompmaker I read the proposal in full but these are two separate features.

@dklompmaker
Copy link
Author

@eyalsk

So not only you're asking to add DI support to the language but you want to add yet another feature to extend interfaces to support it?

My apologies I took this to mean you weren't aware of what was being requested in the proposal. If you think this would be the best approach, I can create a new issue specifically for default interface constructors.

@bbarry
Copy link
Contributor

bbarry commented Jul 3, 2018

Ignoring the whole inject modifier thing...

public container AppContainer
{
     public static IHttpService as HttpService;
     public virtual ISomeType as SomeType;
}

This looks like some sort of DSL for container registry.

Container Behavior Customization

...
Binding with custom factory

Here are 2 different escape hatches for breaking out of the DSL and into today's C#.

This would have to compile down to something similar to how it is done in various AspNetCore libraries. For example in Identity: https://github.com/aspnet/Identity/blob/master/src/Core/IdentityServiceCollectionExtensions.cs

Considering what it would take to build a nontrivial application, libraries all over the place would want to extend an existing container and often would need to utilize that second escape hatch and probably a 3rd that gives raw access to the underlying api somehow.

It seems like a large amount of language complexity for minimal benefit over existing designs that use the raw api without the DSL.

@iam3yal
Copy link
Contributor

iam3yal commented Jul 3, 2018

@dklompmaker Well, part of your proposal suggest to add a feature that doesn't exist today in the language and it shouldn't be tied only to DI because interfaces aren't and so there should be a discussion around what it would mean to have constructors inside interfaces, what's the cons and pros of it and whether it makes sense as such I think that it warrants a new issue.

p.s. I'm not sure whether there's an issue opened/closed about interface constructors so you might want to double check that before you're making a new issue about it.

@theunrepentantgeek
Copy link

You end up writing code that doesn't do anything, factories that you have to write simply exist to inject something that should be automatically injected because you can't use Resolve if you don't want to couple to a DI framework.

This is where the wide ecosystem of DI frameworks comes in.

With Ninject, you don't need to do anything more than define IRoomFactory - you ask Ninject to provide the implementation for you.

I'm sure other DI frameworks have similar functionality - I'm currently coming up to speed with Simple Injector and working out how to do the same thing is on my list of things to learn.

@HaloFour
Copy link
Contributor

HaloFour commented Jul 4, 2018

@leonidumanskiy

How is it handled for you? Can you provide more real-world examples?

Depends on the container. Some support injecting a Func<..., T> as the factory. But I'd be wary of this approach as it is basically service locator. I very rarely inject a factory/locator into a component, and when I do it's usually because whatever container I'm using can't handle circular dependencies (I'm looking at you HK2). I do sometimes bind a factory to an implementation in the container so that I can resolve the dependency using custom logic, but that's pretty rare and usually temporary, like for a percentage-based rollout of a migration.

@asyncawaitmvvm
Copy link

Dependency Injection is disgusting garbage. Keep it far away from anything official. I can name a dozen projects that would have never gotten off the ground if they had to deal with any of these recent anti-pattern buzzwords.

@bbarry
Copy link
Contributor

bbarry commented Jul 4, 2018

@leonidumanskiy

How is it handled for you? Can you provide more real-world examples?

One way to do it is to invert the problem. Instead of requiring a string parameter on the implementation which you pass a runtime constant, require some sort of options class and read a property from it. Then in your container register a created instance of that options class.

@nblumhardt
Copy link

@dklompmaker just to add a different point of view, I think there is ground to be made, here, though it might require going back to the problem-space rather than grafting the functionality of today's containers directly onto the language.

There are really only two facilities provided by IoC containers that don't have simpler solutions already available in .NET languages:

  1. Assembling graphs of loosely-coupled components without fragile, high-churn, hand-rolled composition boilerplate, and,
  2. Implementing deterministic disposal of resources in loosely-coupled component graphs.

Item 1 looks a bit like it might be solved with functional composition, but unfortunately the boilerplate is just as bad there as it is in more OO-styled languages, only with a bit less syntactic ceremony. Item 2 is nasty; attempts to solve it with convention in C# (or C++) are easy to get wrong. It's the irreplaceable "killer feature" that keeps DI/IoC alive in large, complex projects.

Dealing with these issues in the language, or in a language, or even in better IoC framework designs, could move us all forwards while addressing the major downsides of the current approach to DI:

  1. Startup overhead/runtime reflection and code generation have a real performance cost (measured in tens of seconds, for large apps!) Could bringing some work back to build-time improve this (even with partial program knowledge, i.e. when building libraries)?
  2. Structural (object graph configuration) problems like missing dependencies and lifetime misconfigurations are detected late - during testing if you're lucky, at runtime otherwise - Is strong static verification of component graphs possible? Could this be brought back to load or link time?
  3. Tooling support - It's scarily easy to get completely lost in a large IoC-driven codebase; I can F12 to "go to definition", but there's no way to go from a constructor parameter to the actual, correct implementer that will be injected or vice-versa in an IDE. I can visualize how types are statically related, but I can't visualize their composition at runtime. With whole-program knowledge available in an IDE, could static analysis and code navigation be brought into line with the other C# features?

Not sure that this repo/ticket is the place to start this kind of investigation, but dropping a reply in here anyway in case it helps someone crack these problems that have bothered me for just about a decade, now :-)

@leonidumanskiy
Copy link

leonidumanskiy commented Jul 4, 2018

@theunrepentantgeek

With Ninject, you don't need to do anything more than define IRoomFactory - you ask Ninject to provide the implementation for you.

AFAIK Ninject.Extensions.Factory (and any other similar solution) uses Castle.DynamicProxy/Reflection.Emit to create a proxy factory. This does not work on any AOT-only platform where runtime code generation is not allowed. including Xamarin.iOS, Unity3D, Windows.Metro and UWP.

@theunrepentantgeek
Copy link

I disagree that interface constructors wouldn't make sense with DI. Take the following example:
The constructor defines that any ApiService implementation, must take in an IApiConfiguration.

Surely that's a violation of the DRY principle. How a service gets constructed is an entirely separate concern from what functionality that service provides.

Also, if I had an ApiService that didn't require any configuration, why would you force the implementation to accept one?

The whole idea of interface constructors is also very constraining, in ways that would make systems brittle and hard to extend. For example, consider a simple declaration for a logging framework:

public interface ILogger
{
    void Log(LogLevel level, string message);
    ILogger(LogLevel maxLevel);
}

With this definition, implementations of FileLogger, DatabaseLogger would be difficult because they couldn't take the additional parameters required for those uses.

The whole point of interfaces is to allow for additional implementations that can't be anticipated, so mixing concerns like this is an antipattern.

@DavidArno
Copy link

DavidArno commented Jul 4, 2018

From my perspective, dependency injection already exists as as a language feature. Using pure (poor man's) DI, I can inject dependencies into types via constructors and into methods via parameters.

For those that like to assign injection responsibilities to a DI Framework, there exist libraries for that purpose. As they already exist, and as the language already supports DI natively, I see little benefit to muddying the language by building a DI framework into the language. This proposal therefore gets a 👎 from me.

@dylanbeattie
Copy link

On the one hand, this is quite an impressive proposal; the examples and syntax appears to cover a lot of very common use cases, it's well-thought-out and very clearly presented. Which, unfortunately, makes it very easy to conclude that I don't think it's a good idea.

I don't see that it solves any real problem; it's a clean and elegant syntax but I think you compromise a huge amount of flexibility and extensibility for the sake of syntactic sugar. Most of what's proposed is already possible - sure, you need to use [Import] instead of the inject keyword and the syntax isn't quite so clean, but I firmly believe these sort of patterns are better supported by libraries and frameworks than being implemented within the language. So it's a 👎 from me.

@thefringeninja
Copy link

Just use new. That's a 👎 from me.

@iamkoch
Copy link

iamkoch commented Jul 4, 2018

Dependency injection as a language feature

Isn't this just a constructor?

Service locator baked in

I thought the whole service-locator-as-an-anti-pattern debate had long ago been put to bed

Kudos for the well thought-out code samples, but it's putting lipstick on a pig, and Service Locators and DI frameworks are solved problems which don't need to be first-class citizens.

@dklompmaker
Copy link
Author

I appreciate all the great discussion. It seems the community agrees that this proposal doesn't make sense in it's current form. I will close this issue for now. A few final thoughts:

@DavidArno, @dylanbeattie, @iamkoch I appreciate the thoughtful comments.

@theunrepentantgeek I see your point regarding the interface constructors, however I feel like the same argument can be made about default interface methods; which is now in prototype phase. Why should the contract care about the implementation of a method? It seems like the community has already broken from the concept that the interface simply provides a contract. Now an interface could provide some base method body, which imo feels like a worse sin than enforcing a constructor signature.

Thanks everyone!

@HaloFour
Copy link
Contributor

HaloFour commented Jul 4, 2018

@dklompmaker

I see your point regarding the interface constructors, however I feel like the same argument can be made about default interface methods; which is now in prototype phase. Why should the contract care about the implementation of a method? It seems like the community has already broken from the concept that the interface simply provides a contract.

It doesn't break from that concept at all. The contract isn't between the interface and the implementation, it's between the interface and the consumer. That remains intact in that the consumer of the interface is guaranteed to be able to invoke any of the members of the interface. Where those members happen to be implemented is an implementation detail, and not one that the consumer needs to be concerned about (or even aware of).

A constructor, on the other hand, is an implementation detail and the signature of the constructor only makes sense in the context of a particular class. Requiring that implementations have a constructor with a matching signature would severely limit their ability to define their own dependencies, and allowing for consumers of that interface to instantiate the implementation directly through the interface (whatever that means) would mean that the consumer would have to take dependencies on its dependency's dependencies, which leaks further implementation. These are problems best solved through a factory pattern where the consumer takes a dependency on the factory and uses it to create implementations for given parameters.

@HaloFour
Copy link
Contributor

HaloFour commented Jul 4, 2018

@nblumhardt

3 would definitely be welcome in my book, although it might be a bit of a challenge given the ecosystem of IoC containers out there. IntelliJ Ultimate offers this kind of tooling support for Spring where for a given injected dependency you can follow the reference back to the implementation that is expected to be injected. It can even resolve back to methods registered as factories for a service.

1 and 2 pose an interesting concept, although again likely a challenge due to the ecosystem and would necessitate a different solution for each. I could see this being solved through a post-phase (or deployment) step which would run through the bootstrapping of the container, validating the bindings and emitting the results into an intermediate format, perhaps even an assembly. Then you deploy along with this additional metadata and the IoC would use that instead of having to figure out the bindings again.

@bgilner
Copy link

bgilner commented Jul 4, 2018

A proposal to give first-class support to an anti-pattern? No thanks. Might as well require an ESB for a trivial console app...

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

No branches or pull requests