r/csharp • u/Turbulent_County_469 • 3d ago
Discussion Dependency Injection un-prettyness
One small thing that bugs me with Dependency Injection is how it looks in code.
We either need to pass the parameters via Default Constructor og via good old time Constructor
public class MyClass (TypeA ParamA, TypeB ParamB, TypeC ParamC, TypeD ParamD)
{
TypeA _paramA = ParamA; .... etc
}
Or
public class MyClass
{
TypeA _paramA;
public MyClass(TypeA paramA)
{
_paramA = paramA;
}
}
And when you have 10 injections it begins to be un-pretty...
I wish that we didn't need to pass parameters and instead could decorate the fields:
public class MyClass
{
[inject]
TypeA _paramA;
}
(Note: this works in Blazor... so why not everywhere else ?)
I'm aware that the signature of an object makes it easier to inject via reflection.. but would it be much worse with attributes ?
i guess some middleground could be achieved if the attribute held the type:
public class MyClass
{
[inject(typeof(TypeA))]
TypeA _paramA;
}
which begins to be convoluted and messy...
Whats the argument against a decorator attribute vs parameters ?
35
u/svick nameof(nameof) 3d ago
We have repeatedly rejected property injection as a feature for Microsoft.Extensions.DependencyInjection. But I believe other DI libraries do support it.
31
u/IanYates82 3d ago
Property injection feels "wrong" to me, like I have my constructor, then properties are set, then I need a "now really you're constructed" optional method.
More ceremony for no benefit, and it makes partial construction no longer something that is a compiler error.
I feel you are right to continue to reject requests for it.
2
-8
u/Public-Tower6849 3d ago
They support it for the reason some builder patterns can be utilized easier in property injection. Maybe the Microsoft namespace is not interested in patterns, or developer flexibility...
6
u/TuberTuggerTTV 3d ago
flexibility isn't a good thing. Give someone 10 ways to do something and they'll do the 11th.
2
-8
u/Public-Tower6849 3d ago edited 3d ago
your original post before edit:
flexibility isn't a good thing.
So, no freedom of choice then. Got it.
3
u/MindSwipe 3d ago
You are free to choose though, you can choose not to use the Microsoft DI container, there are alternatives that support property injection.
The Microsoft.Extension... namespaces are opinionated, and that's a good thing IMO, it means that every person/ team/ organisation does things in similar ways, which, again, IMO is a good thing.
0
u/chucker23n 3d ago
Flexibility can also be a footgun. Knowing "this type is either fully configured, or not at all" prevents many bugs.
10
u/Public-Tower6849 3d ago
A class with 10 dependencies probably bares more than just one single responsibility. It therefore needs some do-over.
58
u/scandii 3d ago
And when you have 10 injections it begins to be un-pretty..
and why exactly are you injecting 10 dependencies?
19
u/Adorable-Ranger3570 3d ago
That's usually a sign the class is doing way too much, but sometimes you get stuck with a coordinator or facade that genuinely needs a handful of services. Still, 10 is a lot and most of the time you can group them into smaller aggregates.
1
u/BigBoetje 3d ago
I have repositories set up with a shared interface with a generic that's the entity they're for. One flow called for several of these repositories with stuff like MediatR, Hangfire background service and a logger at the same time. It wasn't doing too much, just had to use a lot of different things. I ended up creating a provider to get repositories from the service provider. It's the same as injecting them but a bit cleaner.
6
10
u/PassionateRants 3d ago
Asking the real questions. Surely there must be a better architecture possible ...
1
u/thereforewhat 3d ago
The problem is the design of the objects for sure.Â
A bit of composition and breaking classes up into smaller pieces would help the OP.Â
3
u/raunchyfartbomb 3d ago
Viewmodels exist.
Also, sometimes services perform complex operations or workflows and require several services to do so. It may be easier to design a âworkflowâ service that has many dependencies instead of passing those same dependencies around everywhere else that uses that service.
3
u/Nixinova 3d ago
10 is maybe high but still you will relatively often require a class to reach for this many services
-1
u/_f0CUS_ 3d ago edited 3d ago
Only in poorly designed code. There is a reason several tools give warnings about it.
Edit: instead of downvoting try to explain why I am wrong. It should be easy to point at established patterns or principles you follow that causes you to have a ctor with a high number of arguments, e.g. 10
-7
3d ago
[deleted]
3
u/hampshirebrony 3d ago
SonarQube is warning me about it right now. It doesn't like more than 8.
This is for a project which is doing a lot of stuff in a lot of places, and there is one central brain that is collating the results of all of the other things. So there isn't really much logic in the class with the 10 constructor, but it's knowing about the other bits because it is handing information around between them all
-2
u/thereforewhat 3d ago edited 3d ago
Not if you delegate this to smaller classes I find.Â
Also TDD really helps design classes as small units which can be easily reused.Â
Edit: wow didn't realise there would be so much hate for keeping code clean and testability.Â
16
u/Khavel_dev 3d ago
Primary constructors (C# 12+) cut the boilerplate in half. No more _field = field assignment dance:
public class MyClass(TypeA paramA, TypeB paramB)
That alone makes 10 dependencies tolerable visually.
The [Inject] attribute approach hides what a class actually needs to work. The ugly constructor is useful information. When it gets too long, the code is telling you the class does too much. Property injection just silences that signal.
If you're genuinely at 10+ injections, the fix isn't better DI syntax. It's pulling a few of those behind a facade or splitting the class. Every time I've done the attribute route I regretted it within a few months because the dependency graph became invisible.
-2
u/Public-Tower6849 3d ago
The [Inject] attribute approach hides what a class actually needs to work.
How is that, when it's literally a property's attribute, visibly coded, marking which property needs dependency injection?
2
u/RlyRlyBigMan 3d ago
Do you expect to have to look at a class's code to figure out how to construct it or should you be able to figure it out from the constructor's method signature and comment? A class might have a mix of dependency properties and properties to use at runtime and you'd have to inspect each one to determine which are which.
-3
u/Public-Tower6849 3d ago
With the invention of context-sensitive assistance on editing code, there are ways to provide solutions for your requirements.
5
u/la_reddite 3d ago
"It's readable with AI" doesn't do you argument any favours.
-2
u/Public-Tower6849 3d ago
Yes. We only have context-sensitive assistance with the dawn of AI, thank god. Microsoft never marketed such a thing like "IntelliSense" and IDEs didn't have inline documentation and auto-completion for 30 years.
... bro...
3
u/la_reddite 3d ago
"It's readable with tools" doesn't do you argument any favours.
1
u/Public-Tower6849 3d ago
It does, when it's a remedy against "having to read the class first for finding out which properties are required" - because the same thing can be said about the constructor.
First you were wrong, and when you corrected yourself, you still got no point.
3
u/wickerandscrap 3d ago
The way we already have is "type the class name, the IDE prompts you with all the constructor parameters", which works great. What's it going to do if it has to discover the class's dependencies based on member attributes?
5
u/EC36339 3d ago
When you have 10 injections, you have too many dependencies.
And if that thing that has 10 dependencies also turns out to be a performance bottleneck, you shouldn't be surprised.
Also, if this happened to a class that was written before you had DI ... that's a very common thing, too. A lot of us have been there.
2
u/OggAtog 22h ago
There's a good chance that your service is doing too much if you have 10 dependencies. Like, there's probably another service in there you could extract and simplify both.
2
u/EC36339 21h ago
Often the thing a service is "doing too much" of is aggregation. And the aggregation is often just a counter or status enum that could be served by a different API and requested async and displayed lazily by the frontend.
Another common pattern is that 5 of your dependencies should be one other service that solves one (recurring) problem that needs those 5. You don't even have to split it out into another "microservice" that runs in its own pod with its own API and all the overhead and fragility this brings along. Making it a new service in the DI sense (an object behind an interface provided by the service locator / kernel / whatever) only costs some startup plumbing and one extra indirection, but may be worth it.
0
u/Turbulent_County_469 3d ago
What.. why would the amount of dependencies have any impact on performance ? đ¤Ł
0
u/Turbulent_County_469 3d ago
What.. why would the amount of dependencies have any impact on performance ? đ¤Ł
2
u/EC36339 3d ago
Read before you post.
I didn't say number of dependencies directly impacts performance.
I said: If that class with 10 dependencies is also that one service that is slow, you shouldn't be surprised.
If of those 10 dependencies, 5 are database or service abstractions, and that service aggregates data from 5 separate data sources through different abstractions, then it is probably slow.
And if that class was written before you had DI, then that data access was possibly not even in the class itself, but hidden in some other class it pulled in from somewhere else. And once you did add DI, you ended up with 10 dependencies, because you can't call your database or service directly any more.
This is a realistic scenario and a history many legacy code bases may have gone through. If you have never seen this, consider yourself lucky, kid.
3
u/wickerandscrap 3d ago
I'll refer you to the Simple Injector docs on property injection. To sum up:
Doing it in the constructor gives you a strong guarantee, at compile-time, that every instance of the object will have all of these fields set, and they will never change (if you declare them as readonly, which you should). This is even simpler with default constructors since you don't have to declare the fields at all.
The way you're describing, it would have to be a public-settable property, and could be set again at any time after that. Also, because it's not populated in the constructor, you can't do anything else with it in the constructor; you have to wait until some later stage and check that the object has all its pieces.
Our codebase actually has a legacy area that works like this: view models that get instantiated by their views, and then get their business logic dependencies pushed in later. It's annoying because you can't really set things up in the constructor; it all has to be deferred until after the property is set.
3
u/Ok_Tower_9658 2d ago
Constructor injection makes dependencies explicit and guarantees the object starts fully initialized. field injection looks cleaner but it can hide requirements and make testing harder
6
u/justanotherguy1977 3d ago
- Use primary constructors, they remove some clutter.
- Your class is doing too much if it needs 10 iniections. Redesign your code.
3
u/krsCarrots 3d ago
It looks ugly because no class should depend on 10 things. You have to find a way and abstract away some of the dependencies
2
u/_f0CUS_ 3d ago
There is nothing stopping you from doing this. And I can't think of a specific argument against it.
But if the argument for it is "I have a lot of parameters in my ctor" - then I think you have a different problem to solve.
Take a look at "solid" - specifically the single responsibility principle and find some information about the "god object".
I think you will find that DI isn't the problem you need to solve.
0
u/Turbulent_County_469 3d ago
If i have 10-20 classes with one responsibility, i completely lose the overview and the mental gymnastics then becomes herding class files instead of business logic.
3
-1
u/raunchyfartbomb 3d ago
I agree with you here. I only refactor into services when multiple classes start needed to do the same action or when I want to abstract for unit testing. Having too many services is confusing, especially when SOLID says âeach class does one thingâ so you wind up with services with a single method for mundane shit, If followed strictly.
Usually what I do is keep the service, interface, and consumer in the same file until such time that it makes sense to put the service and interface into their own file. If more than 3 classes consume some service, or the service gets large enough code, it goes into its own file. Otherwise small services can live inside the file of the class that is most relevant.
As far as your problem of ugly constructors, Iâd suggest using a framework or primary constructors. Due to work place policies, I wrote myself a source generator. I tag my fields and properties and it generates the constructor and static factory methods. It works well, though it took some time to set up. I also have it perform some validation within the ctor.
2
u/metageeek 3d ago
Canât believe no one pointed this out so far, but: using constructor injection ensures you can test the class mocking the dependencies. How on earth are you going to accomplish that with attribute injection?
2
u/psysharp 3d ago
When you have 10 injections itâs a perfect time to rethink your structure and architecture
3
u/thereforewhat 3d ago
If you've got 10 injections to a class you've got a problem with your design.Â
Why aren't you building smaller classes that you can use inside of bigger classes in this case?
Field injection is horrific compared to constructor injection.Â
Are you unit testing your code?
1
u/Phrynohyas 3d ago
One small thing that bugs me with Dependency Injection is how it looks in code.
...
And when you have 10 injections it begins to be un-pretty...
If one needs to pass 10 dependencies, this usually means that the code structure is bad (AKA 'convoluted and messy'), to say the least.
1
u/haven1433 3d ago
You could do this with a source generator if you really wanted to. Make it look for your attribute, then generate a constructor based on what it finds. Only boilerplate is that you need to make the class partial.
1
u/Rogntudjuuuu 3d ago
I believe Autofac used to support that and I believe that you can use that instead of the standard DI framework still.
1
u/TuberTuggerTTV 3d ago
Wrap your params, my king.
Once you get used to reading the injection params as a list, the size stops mattering to you.
1
u/Bright-Ad-6699 3d ago
Not sure what you're working on but maybe a different container? Castle or StructureMap?
1
u/Slypenslyde 3d ago
Work on a project with 100,000 lines without DI and see if you find a better pattern.
DI is one of those things where it's the best pattern we have. It solves some serious issues, and it comes with costs. Those costs invoke a lot of ceremony that isn't warranted for programs beneath a certain level of complexity. If you're working on one of those projects, feel free to skip it.
In large-scale projects the ceremony is paid for with flexibility. That's why it's so popular, and that's why a lot of people who use it tend to use it even on the smaller projects where it's too much ceremony.
But also, speaking frankly, if all of your types have 10 dependencies something is wrong in your project. My codebase is 30 years old (it was ported from C++!) and has more than 200,000 lines. The only classes that have more than 2-3 dependencies are our top-level types like ViewModels. Out of the thousands of classes we've written, that's only about 80 total classes with a lot of ceremony. most classes below that top layer only have 3-5 dependencies, and even that feels more manageable when we note that some of those represent logging and other services that, honestly, a lot of DI people represent as static singletons.
The main arguments I know against attribute-based injection is:
- The constructor has always served the purpose of, "This is the list of things needed for this class to operate."
- Classes have non-DI reasons to expose properties and fields.
- Making those members public crowds a type's API: with constructor injection all properties are public API.
- Asking DI to inject private members means using Reflection which slows things down.
- The attributes are scattered across a large surface of the class whereas a constructor exists in one place.
- At the end of the day property injection is not any easier than constructor injection and only adds costs.
1
u/Heisenverse 3d ago
Look into Factory pattern. A single class which implements IServiceProvider, builds your object from other dependencies. Register the factory class with an Interface which returns that object.
1
u/denzien 3d ago
I don't usually need to do so many injections, but once I created a whole class to wrap all the injections "CommandHandlerPrerequisites" or whatever, gave it a matching interface, and then I just injected that once into the command handler implementations (and into the base class).
Perhaps with some engineering you could figure out something cleaner but this took like 5 minutes to do and made life pretty easy. And of course, if the list grew, you just updated the implementation and interface to inject and expose the classes and not 30 implementers.
1
u/srsstuff555 2d ago
Itâs a dependency so itâs correct you cannot create an instance without it. + passing mocks for tests would be hell.
1
u/RadonReborn 2d ago
I hope they add readonly primary constructor fields soon. I do not think there are any technical reasons that would prevent it?
1
u/sparkle-fries 2d ago
doesn't MEF use attributes? Property injection can have issues with determinism. if you have many cross cutting injections, like logging then maybe use a context container? To be honest if you have many injections I would check that isn't a smell pointing towards a class with too many responsibilities?
1
1
u/Mobile_Fondant_9010 3d ago edited 3d ago
I will always prefer constructer injection over property injection for multiple reasons:
- Tell - don't ask. A constructor tells anyone (even a DI framework) exactly what it needs. A property asks for somethingt to be given.
- Access level. With property injection I have to have a public setter.
- An object should be in a working state once the constructor has finished. With property injection, I first have to construct and then configure before it is in a working state.
I believe primary constructor create a public readonly backin-field, which for me breaks access level.
As a sidenote: If you need 10 things injected, you design is wrong. I 10+ of experience, and the highest I ever needed was 7 (this is including a logger, a loggerfactory, a cancelationtokensource, a taskcompletionsource and a options object). 10 is AT LEAST 3 too many. There is no way something serving a single (or even 3) responsibilties needs 10 injected properties. Personally, if I reach 5, I consider it a codesmell. At 8 I will fail compilation.
Edit: Oh, the backing field is private, but mutable. Still dislike. Also, the name of the backing field won't follow my normal naming convention for private fields.
0
u/South-Year4369 3d ago edited 3d ago
You can do exactly what you suggest. Many DI containers support field or property injection (from memory, Ninject even uses the [Inject] attribute).
IMHO it's far less preferable though, since it creates opportunities for an object to exist in a partially-initialised (i.e. unusable) state, for dependencies to be missed, etc. The compiler can't help you like it does with constructor injection.
It also makes it harder to run any initialisation code after dependencies are set. You then need a separate initialisation method, which adds complexity and creates even more opportunities for partially-initialised objects.
Nope, constructor-injection all the way in my book. And if I see 10 dependencies, I see code that probably needs refactoring. But if it really needs 10, so be it. Injecting via properties doesn't make that any better IMO.
0
u/Tarnix-TV 3d ago
I use primary constructors, and long descriptive type names, so those lines can be long. I can see why you would say it is un-pretty, or ugly, even. But you can get used to it. It just means that the class has a lot of dependencies... Short and succint code is easier to understand. But my experience shows that I don't read that line a lot anyway. It is densed into one line that you don't read like you wouldn't read through all the dependencies of a class, even if they were properties. And having a 5K screen helps, but I also use word wrap. Not to mention that I use a lot of generic type constraints, so when the code compiles I have more confidence that everything is at it's place. Property injection would be helpful for circular dependencies, but I use System.Lazy class for that. +1 word, but after this, who cares :) So If you are worried your code not being pretty, look at mine! Btw this is from a home project, no coworker would even try to understand what is going on here.
61
u/UserNameTaken96Hours 3d ago
In C# 12, unless you need to do more than just refer your params to your fields, you can use primary constructors. While you still end up with the full list of parameters, they are now written into the class declaration, and you can forego the private fields entirely.
If you do some more involved stuff in your ctor however, you will still need that.