I don't like it. The main disadvantage is that the decorator must know what it is decorating (by specifying a key to a 'FromKeyedServicesAttribute'). What if we want to have multiple implementations of some interface and all of them must be decorated in the same way? In my opinion it is indirect dependency by a string key. Technically it does not differ from extending a class instead of implementing the same interface and it's even worse because the dependency is hidden in a parameter of some attribute.
I believe the key belongs to and should be defined within the decorator. So it doesn't know what it is decorating, it just creates a requirement that something must be keyed with its key in order for the decorator to be used. Multiple different decorators of the same interface all define their own unique keys (I suggest a public const guid string). Then in the bindings you'd choose which decorator implementation and use its key for the keyedsingleton You're right that specifying the key outside of the decorator is not really much different to injecting the implementation itself. I think not defining the key string within the decorator class is a major oversight by this video
Yea, I don’t really see an issue with the first DI syntax. I think it quite succinctly describes what is happening. If your trying to constrain the object your decorating you should really do that through interfaces.
It's coupling the composition root with the implementation; much better to use named registrations and in turn register the decorator using a factory delegate (that in turn resolves the named registration). Regardless, the decorator pattern is one of the most productive and important patterns in modern API development.
this is the comment i was looking for. im also not a fan of needing to bake in anything DI related to any classes or interfaces. anyone could switch to autoFac or lamar, etc and now what registrations are broken
This api doesn't really work well with decorator pattern since it supports only one level of decoration. The whole point of decorator is that you can stack as many of them as you want. Edit: I have an idea: services.Decorate() .With() .ThenWith() .ThenWith();
I really dislike the way you hook up the decorated service in the implementation. That breaks the whole idea of having your application composition separate from your implementation. You're better off putting that in your extension method, resolve the og there. But you can't really do that with the vanilla DI framework, because your factory needs to be aware of all the parameters while most of the time your decorator will want some other constructor parameters too.
In my humble opinion, the fact that we have to go through all these hoops and complexity just to get it to work screams KISS to me. Do it the way we did it before all these dependency injection frameworks. Instantiate your objects and pass them to the ctors. Easy. We see the flow. Less lines of code. The principle of KISS.
Decorators = method interception = pipelines = all good. I don't buy the developer confusion argument against decorators, I think the problem can be overcome with structuring and awareness of configured behaviors. What I don't like about this example is the fixed key that the decorator needs to know, which limits the decorator to a single depth because of a dependency on the original/core implementation. Decorators as behavioral extension points should be capable of arbitrary depth.
You can always calculate a key based on the redirection flow so compose a string that is IWeatherService-Resilient-OpenWeatherMap. This is just an example of the approach. You can take it and adapt it to whatever you want
I'm not sure when is good to have the decorator pattern, and when is not. Recently I begin to work on an new company where they were using the Decorator Pattern for translations where basically are 5 layers to find translations (custom -> providers -> another providers -> etc) and was really hard to debug and understand completely the code. For this kind of escenarios such as retry policies or cache layer I think is ok.
At my team we use decorators when we want to enhance logic of libraries/frameworks. Since most of them use dependency injection, it’s easy to add your own code to the one you don’t have control over.
You’ve touched on a downside of decorators. If you get too carried away with them, they can obfuscate code. Not that surprising, getting carried away with patterns/abstractions almost always leads to a poor cost to benefit trade off. Generally, you want to reserve them for cross cutting “operational” concerns and not have them too heavily nested. They normally aren’t a good fit for business logic.
Sounds like decorators were used where they shouldn't be used. But for stuff like cross cutting concerns like logging it's actually a nice pattern to apply l
Decorator pattern is very useful on food order logic. Each extra (or removed condiment) is wrapping around the main item and is treated as that item also. Figure a Russian doll.
Amazing video. I have a suggestion to even avoid 2 issues with this approach: 1. To avoid using hardcoded keys to be passed in the registration, we can add a static Key property in the Interface. This will allow to pass the Key property instead. i.e. services.AddDecoratedSingleton(MyService.Key), something like that. 2. To avoid boxing, why not using generics. In this case the Interface may be of type generic, so the Key property I mentioned before may be of type K.
My preferred decorator pattern involves using DispatchProxy. That way I can (e.g.) wrap all calls in an exception logger, or timing, or security without having to implement all the interfaces I want to decorate. It will handle any method on the interface using it automatically. The hardest part if elegantly registering it all (still working on that aspect of it). I'm sure that since it's using reflection, it's not the fastest, but the laziness aspect really pays off.
Great video. The main issue is coupling introduced by the key which kills the idea of decoration as you're now tight to a single implementation. I believe keeping it explicit in the DI registration or using a factory is better
A generic retry method that accepts an Action along with a retryCount and retryDelay (think milliseconds Timespan) is much more reusable in all kinds of situations. This feels like an antipattern as far as decorators go.
What about using generics instead of string keys...? So the IService will have a IService which gets registered to the actual implementation, then your IService can be registered with your "root entry" actual implementation. There can be many variations using this pattern
So using standard m$ DI u don't get much out of the box for some advanced dependency injection scenarios but if u switch to Autofac things changes drastically as u get some goodies from first day. Personally I wouldn't go with any attributes for DI to not polute classes with some messy attributes, so what is where injected it is a responsibility on registration level in DI container.
We've been able to do this in a much cleaner way with Autofac and other DI containers for a long time. Microsoft needs to implement it in their DI container.
I'm not sure if using the keyed service really is cleaner as you say. Your decorated service now needs to know about the key. Is that really an improvement?
We usaully do this kind of decoration for caching the results of the actual service. IMHO this is very clean and you dont pollute the original service with stuff like caching, retrying and such.
Seems to me that Factory Pattern is also a fit solution for resolving dependencies of a decorated service like this. Correct me if I'm wrong. I'm still learning design patterns and those are really great to use :D
This seems like it would be good for logging, retries, audit trails, and other non critical pieces of functionality. I wouldn't expect this type of approach would necessarily be useful or even ideal for business logic. As you pointed out, the main service should really only know how to do it's main job and the rest of the fluff can be moved elsewhere and this approach could work for that. I think it may get a little out of control if you have multiple different kinds of fluff that need to be removed. Logging would need it's own layer, retries their own layer, caching, etc... The more fluff you have, the more layers you have which can just make the code much harder to understand I think.
So what if I want to decorate a class with multiple decorations? Right now the decoration receives the implementation via DI in the constructor. What would the second decorator get? Must be the first decoration right? At this point what's the difference to the Chain of Responsibility pattern other than the junctioning to the IServiceProvider?
I guess the example here is little hard to digest. I can see one useful scenario while api versioning(for V1 i can have one implementation and for V2 i can have different implementation). Thanks for sharing it :)
As said in other comments, I prefer to do this kind of extension with Delegates, it's much more decoupled and easier to read and maintain. 🙂 Just need to be careful with memory allocations. 🙂
When using design patterns, do we really need to follow what it tells? Or just use some parts of it. Because from what I have read, the participants for this pattern are all classes and no Interface.
I'm a little confused what are the benefits over inheriting the class and overriding a virtual method? plus you have to re:implement again the whole interface....
Please just give us a master video on Dependency Injection already xD I'm talking factories, required services, .. etc. basically a compilation video of "good practice and cool things" you can do with DI in C#
what if there are more implementations and I want to inject sometimes an implementation, and other times another implementation? should I register every combination? just like a strategy pattern
9:36 this is edging kinda close to the service locator antipattern / just injecting the implementation. It is crucial that the ResilientWeatherService decorator implementation defines and owns its decorator key - i.e., that "og" needs to be defined within the ResilientWeatherService, and unique in the system (I suggest using a const GUID)
In C# 11 with virtual static members in interfaces you could do it in a way that that is strongly typed, you could also do by key. Example: public interface IServiceDecorator where TService : class { public static abstract TService Decorate(TService serviceToDecorate); } public static class ServiceExt { public static void AddDecoratedSingleton(this Services services) where TService : class where TImplementation : TService where TServiceDecorator : TService, IServiceDecorator { services.AddSingleton(); services.AddSingleton(locator => TServiceDecorator.Decorate(locator.GetRequiredService())); } } public class ResilientWeatherService : IWeatherService, IServiceDecorator { public static IWeatherService Decorate(IWeatherService toDecorate) => new ResilientWeatherService(toDecorate); private readonly IWeatherService _decoratedService; private ResilientWeatherService(IWeatherService realService) { _decoratedService = realService; } public Task GetWeatherInCityAsync(string cityName) { return _decoratedService.GetWeatherInCityAsync(cityName); } } But this still only supports one decorator.
@@nickchapsas C#11's static abstract interface members: public interface IServiceDecorator where TService : class { public static abstract TService Decorate(TService serviceToDecorate);
public static abstract Guid ServiceKey { get; } } public class ResilientWeatherService : IWeatherService, IServiceDecorator { private const string SERVICE_KEY = "F5E3314A-E75E-4D8D-A5E0-3A1840FCF899"; public static Guid ServiceKey { get; } = new Guid(SERVICE_KEY);
public static IWeatherService Decorate(IWeatherService toDecorate) { return new ResilientWeatherService(toDecorate); } private readonly IWeatherService _decoratedService; public ResilientWeatherService([FromKeyedServices(SERVICE_KEY)] IWeatherService realService) { _decoratedService = realService; } public Task GetWeatherInCityAsync(string cityName) { return _decoratedService.GetWeatherInCityAsync(cityName); } } public static class ServiceExt { public static void AddDecoratedSingleton(this IServiceCollection services) where TService : class where TImplementation : TService where TServiceDecorator : TService, IServiceDecorator { services.AddSingleton(); services.AddSingleton(locator => TServiceDecorator.Decorate(locator.GetRequiredService())); }
public static void AddKeyedDecoratedSingleton(this IServiceCollection services) where TService : class where TImplementation : TService where TServiceDecorator : TService, IServiceDecorator { services.AddKeyedSingleton(TServiceDecorator.ServiceKey); services.AddSingleton(); } }
This was very cool. I always learn something. I will try this out today. Thanks Nick.
ปีที่แล้ว +8
IMHO keyed services increase complexity in a place where most of the stuff is 'hidden' from the developer (DI) so it may be hard to understand what is going on in a larger project. I do not like the KeyedService attribute on the constructor .. but it is probably fine in a project where this is common and developers understand the paradigm. I personally would avoid placing the original (final) service in the DI completely. Even in the first example I would not use x.GetService to get the original (final) service but I would 'new' it within the method where the decorator service is created.
It's like everybody is playing the floor is lava, but where the rule is "avoid new() at all costs". The whole ecosystem is full of these kinds of things. Automapper: avoid writing a mapping function, which I kind of get honestly. Then there is fluentvalidation, which forces you into relying on reflection and interfaces just so you can avoid writing if statements in favor of chaining methods. We just really don't like 'simple' code.
Great video Nick. In my case, i believe in this specific situation is better two importations of same interface with the common code in an abstract class. Of course for this is required skip the restriction of do not modify the weather service 😂
This seems to result in a situation where it would be technically "correct" or "legal" to inject the ResilientWeatherService as the dependency to itself since the only requirement is that it implements IWeatherService, which of course would end up in rather strange behavior. So what ResilientWeatherService actually requires to function properly is a "real" WeatherService, and that's not really evident from the code and neither is what is a "real" WeatherService unless you dig into the implementations. Seems bit more of a clever than a good solution.
I am looking for this, but then a "genric" variation, e.g. a class that retries all functions of an interface. Not sure how to do this tough, maybe some proxy class/castle core can do this.
This is not quite correct. "Wrapper" is a more generic term used for different patterns, but most of the time it is used as a synonym for the adapter pattern. And: Decorator and adapter are two different patterns. The decorator exposes the same interface as the inner service, the adapter exposes a different interface.
I don’t think this is good practice. Decorators shouldn’t care about the order, by doing this you’re adding knowledge of the order in which they’re used i.e. the resilient service always wants the “og”? What if you want to add a second third fourth decorator? Improper use of the keyed services imo. What if you wanted to change the order I.e. before you had logging > resilient > og and you want to swap logging and resilient over? Then you’re violating the open/closed principle that you originally made the decorators to solve by editing class code, instead of simply changing your composition in the app root.
I don't like this approach. In the end, it doesn't save a registration to the DI container, and ends up coupling the decorator to a specific instance through a magic string. High coupling without type integrity seems to me like the worst of both worlds. I find that the FromKeyedServicesAttribute to be an easy trap to fall into if you're just going to use it as-is in your code. That being said, I can see more interesting uses for it with codegen libraries that would produce proxies, such as an AOP framework.
But then you have to reference that key in the class that needs that implementation, and you have to do that in a static way. So you can't auto-generate that key. You might as well not use keys at all then, and create an extension method that wraps the original key-less code, I guess.
I call bs. Using the key locks you into that key, can't swap out the `OpenWeatherService` against the `WewtherComWewtherService` without modifying the `ResiliantWeatherService`. This impl is basically a 180° rotation back to the start but a bit cleaner. The `IWeatherService` is for consumption. The `IRawWeatherService` is for the API call. `IBaseWeatherService` has all the members. Both `IWeatherService` and `IRawWeatherService` inherit from `IBaseWeatherService`. The `ResilientWeatherService` implements `IWeatherService` and takes `IEnumerable` via DI. The resilient service can now do 1. Retying 2. Chain if responsibility 3. ... This is one of the few cases where we throw multiple interfaces into one file. Annotate the `IBase*` with some do not use directly warning or mark as obsolete, and suppress in in the other interfaces.
I can see this getting confusing during a code review, and I would reject this if I saw it come through for anything that wasn't a wrapper for something we didn't have access to. Even then, I'm going to demand that either the DI be set up in a more readable way, or the service naming be clearer that it's a wrapper, and not run the risk of accidentally injecting itself infinitely if the DI setup is slightly wrong. This falls under "clever code" and should be rejected. Clever might not be bad, but if you come back to it 6 months later, are you going to know exactly what's going on at first glance? The video had to explain it, and it still doesn't "add behavior" to existing classes, so much as wraps them. "Adding behavior" is an extension method, not a wrapper. I might be arguing semantics here, but spending as much time as I do fighting with code that was written like this over the course of years has given me a serious distaste for clever code that conceals something in some way. And the way the injection is set up here fits the bill perfectly. If you're wrapping a concrete implementation, inject the concrete implementation. Make the consumers of that implementation use the interface. Be explicit about it.
Why do MS make things so obscure. I much prefer AutoFac’s RegisterDecorator approach. The method gives clear meaning and you don’t need to specify a f’ing key.
Every comp. science students should watch Nick's vides imo. You're not going to learn programming, instead improve your vision. As Cse students the least thing we do is coding at school, we learn a lot of theory etc. And thats the youtube channel where you how to bind theory into code. I'm recommended your videos not only for this kinda videos but low-level understanding videos like the ones with Spans etc. People like Nick, sees a new feature, thinks differently, and be like "What if I use it for this also?" and this kind of vision-improving videos comes. Thank you master :)
No! This does not allow you to decorate further, it's now locked. Say you wanted an analytics decorator between the decorator and the decoratee? You could not do that without modifying the decoratee. Also, the decoratee should not know it's being decorated.
I don't like this approach. It seems to be distributing the knowledge over what needs injected into multiple places because now the decorator service needs to know the attribute and the key to use for the injected service, in addition to having this all still controlled at the service registration point. IMO it's just better to stick to the service registration and using a factory there. It's cleaner and keeps the knowledge of how the service decorator is set up in one place.
This is just wrong. By using key-based injection attribute in a service You clearly broke inversion of control paradigm in a way that the service now is aware of DI logic and you made it coupled to DI. Smells like an evident anti pattern. The service should not give a shit about injected dependecies creation details and DI mechanics related to that.
I don't like it. The main disadvantage is that the decorator must know what it is decorating (by specifying a key to a 'FromKeyedServicesAttribute'). What if we want to have multiple implementations of some interface and all of them must be decorated in the same way? In my opinion it is indirect dependency by a string key. Technically it does not differ from extending a class instead of implementing the same interface and it's even worse because the dependency is hidden in a parameter of some attribute.
This
I believe the key belongs to and should be defined within the decorator. So it doesn't know what it is decorating, it just creates a requirement that something must be keyed with its key in order for the decorator to be used.
Multiple different decorators of the same interface all define their own unique keys (I suggest a public const guid string). Then in the bindings you'd choose which decorator implementation and use its key for the keyedsingleton
You're right that specifying the key outside of the decorator is not really much different to injecting the implementation itself. I think not defining the key string within the decorator class is a major oversight by this video
Yea, I don’t really see an issue with the first DI syntax. I think it quite succinctly describes what is happening. If your trying to constrain the object your decorating you should really do that through interfaces.
Yes, I was thinking the same I'm sure this must violate at least one (I..e Liskov), if not more, of the SOLID principles.
I agree with all of you. IMHO readability is way better in the first example. Dependency injection by magic string sound error prone way to do it.
I don't like the key approach with the decorator owning the key. It would rapidly become a mess with multiple level of decorator.
It's coupling the composition root with the implementation; much better to use named registrations and in turn register the decorator using a factory delegate (that in turn resolves the named registration). Regardless, the decorator pattern is one of the most productive and important patterns in modern API development.
this is the comment i was looking for. im also not a fan of needing to bake in anything DI related to any classes or interfaces. anyone could switch to autoFac or lamar, etc and now what registrations are broken
5:09 - For anyone even considering using labels like that, instead just use a do-while with a break condition.
Or just a for loop with a return. I didn’t even know about labels in c# 😅
This is a bit like Inception, except you never go more than 1 level. Any deeper is a path to madness...
This api doesn't really work well with decorator pattern since it supports only one level of decoration. The whole point of decorator is that you can stack as many of them as you want.
Edit: I have an idea:
services.Decorate()
.With()
.ThenWith()
.ThenWith();
It's better to use scrutor at that point
there is a lib that does that, Scrutor
@@finickyflame ups, i didnt see this
I approve of this message 😜👍
@@khellang I think you might be a little bit biased ;)
I really dislike the way you hook up the decorated service in the implementation. That breaks the whole idea of having your application composition separate from your implementation.
You're better off putting that in your extension method, resolve the og there. But you can't really do that with the vanilla DI framework, because your factory needs to be aware of all the parameters while most of the time your decorator will want some other constructor parameters too.
In my humble opinion, the fact that we have to go through all these hoops and complexity just to get it to work screams KISS to me. Do it the way we did it before all these dependency injection frameworks. Instantiate your objects and pass them to the ctors. Easy. We see the flow. Less lines of code. The principle of KISS.
Decorators = method interception = pipelines = all good. I don't buy the developer confusion argument against decorators, I think the problem can be overcome with structuring and awareness of configured behaviors.
What I don't like about this example is the fixed key that the decorator needs to know, which limits the decorator to a single depth because of a dependency on the original/core implementation.
Decorators as behavioral extension points should be capable of arbitrary depth.
You can always calculate a key based on the redirection flow so compose a string that is IWeatherService-Resilient-OpenWeatherMap. This is just an example of the approach. You can take it and adapt it to whatever you want
I'm not sure when is good to have the decorator pattern, and when is not. Recently I begin to work on an new company where they were using the Decorator Pattern for translations where basically are 5 layers to find translations (custom -> providers -> another providers -> etc) and was really hard to debug and understand completely the code. For this kind of escenarios such as retry policies or cache layer I think is ok.
At my team we use decorators when we want to enhance logic of libraries/frameworks. Since most of them use dependency injection, it’s easy to add your own code to the one you don’t have control over.
You’ve touched on a downside of decorators. If you get too carried away with them, they can obfuscate code. Not that surprising, getting carried away with patterns/abstractions almost always leads to a poor cost to benefit trade off.
Generally, you want to reserve them for cross cutting “operational” concerns and not have them too heavily nested. They normally aren’t a good fit for business logic.
Sounds like decorators were used where they shouldn't be used.
But for stuff like cross cutting concerns like logging it's actually a nice pattern to apply l
We decorate classes for caching using Scrutor decorator attribute.
Decorator pattern is very useful on food order logic. Each extra (or removed condiment) is wrapping around the main item and is treated as that item also. Figure a Russian doll.
I have watched most of your videos. I am not sure that I agree with the philosophy of this but the idea to do it this way is brilliant. Keep it up.
Amazing video. I have a suggestion to even avoid 2 issues with this approach:
1. To avoid using hardcoded keys to be passed in the registration, we can add a static Key property in the Interface. This will allow to pass the Key property instead.
i.e. services.AddDecoratedSingleton(MyService.Key), something like that.
2. To avoid boxing, why not using generics. In this case the Interface may be of type generic, so the Key property I mentioned before may be of type K.
My preferred decorator pattern involves using DispatchProxy. That way I can (e.g.) wrap all calls in an exception logger, or timing, or security without having to implement all the interfaces I want to decorate. It will handle any method on the interface using it automatically. The hardest part if elegantly registering it all (still working on that aspect of it).
I'm sure that since it's using reflection, it's not the fastest, but the laziness aspect really pays off.
Great video. The main issue is coupling introduced by the key which kills the idea of decoration as you're now tight to a single implementation. I believe keeping it explicit in the DI registration or using a factory is better
A generic retry method that accepts an Action along with a retryCount and retryDelay (think milliseconds Timespan) is much more reusable in all kinds of situations. This feels like an antipattern as far as decorators go.
What about using generics instead of string keys...? So the IService will have a IService which gets registered to the actual implementation, then your IService can be registered with your "root entry" actual implementation. There can be many variations using this pattern
So using standard m$ DI u don't get much out of the box for some advanced dependency injection scenarios but if u switch to Autofac things changes drastically as u get some goodies from first day. Personally I wouldn't go with any attributes for DI to not polute classes with some messy attributes, so what is where injected it is a responsibility on registration level in DI container.
We've been able to do this in a much cleaner way with Autofac and other DI containers for a long time. Microsoft needs to implement it in their DI container.
The goto made me cry :)
I'm not sure if using the keyed service really is cleaner as you say. Your decorated service now needs to know about the key. Is that really an improvement?
We usaully do this kind of decoration for caching the results of the actual service. IMHO this is very clean and you dont pollute the original service with stuff like caching, retrying and such.
Used to have decorators done but was using Autofac. It was really simple
How do you feel about using Scrutor for decorating instead?
Seems to me that Factory Pattern is also a fit solution for resolving dependencies of a decorated service like this. Correct me if I'm wrong. I'm still learning design patterns and those are really great to use :D
I keep using scrutor library for decorating things, but for 1 lvl of decoration it is nice feature
We've been using Scrutor as well. Very easy to use.
What about extension methods Nick , they also can modify behavior of a class without touching it ?
This seems like it would be good for logging, retries, audit trails, and other non critical pieces of functionality. I wouldn't expect this type of approach would necessarily be useful or even ideal for business logic. As you pointed out, the main service should really only know how to do it's main job and the rest of the fluff can be moved elsewhere and this approach could work for that. I think it may get a little out of control if you have multiple different kinds of fluff that need to be removed. Logging would need it's own layer, retries their own layer, caching, etc... The more fluff you have, the more layers you have which can just make the code much harder to understand I think.
So what if I want to decorate a class with multiple decorations? Right now the decoration receives the implementation via DI in the constructor. What would the second decorator get? Must be the first decoration right? At this point what's the difference to the Chain of Responsibility pattern other than the junctioning to the IServiceProvider?
I don't like adding the FromKeyedService attribute to the implementation class, because now this class is dependent on a DI container.
I guess the example here is little hard to digest. I can see one useful scenario while api versioning(for V1 i can have one implementation and for V2 i can have different implementation). Thanks for sharing it :)
As said in other comments, I prefer to do this kind of extension with Delegates, it's much more decoupled and easier to read and maintain. 🙂 Just need to be careful with memory allocations. 🙂
When using design patterns, do we really need to follow what it tells? Or just use some parts of it. Because from what I have read, the participants for this pattern are all classes and no Interface.
9:10 needs to receive more love! :D
Don't let the haters tell you that this isn't a valid case for goto :D even nicer when you change it to catch (Exception) when (retryCount
I use scrutor for decorated dependency at the moment. I wish this will be supported in MS DI.
For the retry logic I much prefer wrapping the trial in a while (true), and breaking the loop by throwing the exception after retrying too many times
I'm a little confused what are the benefits over inheriting the class and overriding a virtual method? plus you have to re:implement again the whole interface....
Nick you are og, i love it.
This is interesting for extending sealed classes for other things i would probably just use inheritance
Please just give us a master video on Dependency Injection already xD I'm talking factories, required services, .. etc. basically a compilation video of "good practice and cool things" you can do with DI in C#
what if there are more implementations and I want to inject sometimes an implementation, and other times another implementation? should I register every combination? just like a strategy pattern
Or just do an extension on the interface to do the retry logic.
As a demo sure this is fine to show something but not a fan of the method
9:36 this is edging kinda close to the service locator antipattern / just injecting the implementation.
It is crucial that the ResilientWeatherService decorator implementation defines and owns its decorator key - i.e., that "og" needs to be defined within the ResilientWeatherService, and unique in the system (I suggest using a const GUID)
In C# 11 with virtual static members in interfaces you could do it in a way that that is strongly typed, you could also do by key. Example:
public interface IServiceDecorator where TService : class
{
public static abstract TService Decorate(TService serviceToDecorate);
}
public static class ServiceExt
{
public static void AddDecoratedSingleton(this Services services)
where TService : class
where TImplementation : TService
where TServiceDecorator : TService, IServiceDecorator
{
services.AddSingleton();
services.AddSingleton(locator => TServiceDecorator.Decorate(locator.GetRequiredService()));
}
}
public class ResilientWeatherService : IWeatherService, IServiceDecorator
{
public static IWeatherService Decorate(IWeatherService toDecorate) => new ResilientWeatherService(toDecorate);
private readonly IWeatherService _decoratedService;
private ResilientWeatherService(IWeatherService realService)
{
_decoratedService = realService;
}
public Task GetWeatherInCityAsync(string cityName)
{
return _decoratedService.GetWeatherInCityAsync(cityName);
}
}
But this still only supports one decorator.
The key is a problem. How about it being the namof the implementation class?
Yeah could totally be that as long as it’s unique
@@nickchapsas C#11's static abstract interface members:
public interface IServiceDecorator where TService : class
{
public static abstract TService Decorate(TService serviceToDecorate);
public static abstract Guid ServiceKey { get; }
}
public class ResilientWeatherService : IWeatherService, IServiceDecorator
{
private const string SERVICE_KEY = "F5E3314A-E75E-4D8D-A5E0-3A1840FCF899";
public static Guid ServiceKey { get; } = new Guid(SERVICE_KEY);
public static IWeatherService Decorate(IWeatherService toDecorate)
{
return new ResilientWeatherService(toDecorate);
}
private readonly IWeatherService _decoratedService;
public ResilientWeatherService([FromKeyedServices(SERVICE_KEY)] IWeatherService realService)
{
_decoratedService = realService;
}
public Task GetWeatherInCityAsync(string cityName)
{
return _decoratedService.GetWeatherInCityAsync(cityName);
}
}
public static class ServiceExt
{
public static void AddDecoratedSingleton(this IServiceCollection services)
where TService : class
where TImplementation : TService
where TServiceDecorator : TService, IServiceDecorator
{
services.AddSingleton();
services.AddSingleton(locator => TServiceDecorator.Decorate(locator.GetRequiredService()));
}
public static void AddKeyedDecoratedSingleton(this IServiceCollection services)
where TService : class
where TImplementation : TService
where TServiceDecorator : TService, IServiceDecorator
{
services.AddKeyedSingleton(TServiceDecorator.ServiceKey);
services.AddSingleton();
}
}
This was very cool. I always learn something. I will try this out today. Thanks Nick.
IMHO keyed services increase complexity in a place where most of the stuff is 'hidden' from the developer (DI) so it may be hard to understand what is going on in a larger project. I do not like the KeyedService attribute on the constructor .. but it is probably fine in a project where this is common and developers understand the paradigm. I personally would avoid placing the original (final) service in the DI completely. Even in the first example I would not use x.GetService to get the original (final) service but I would 'new' it within the method where the decorator service is created.
It's like everybody is playing the floor is lava, but where the rule is "avoid new() at all costs". The whole ecosystem is full of these kinds of things. Automapper: avoid writing a mapping function, which I kind of get honestly. Then there is fluentvalidation, which forces you into relying on reflection and interfaces just so you can avoid writing if statements in favor of chaining methods. We just really don't like 'simple' code.
@@z0nxThis made my day!
I think we can use scrutor nuget package to decorate in a simpler way
Great video Nick. In my case, i believe in this specific situation is better two importations of same interface with the common code in an abstract class. Of course for this is required skip the restriction of do not modify the weather service 😂
Oh yes! Decorating pattern holy wars in comments!
Really don't like the key based approach, mainly because of the introduction of a string and the dependency on the attribute.
Use a decorator base class and Autofac. Base class adds pass thrus, decorators only have to override methods they care about.
This seems to result in a situation where it would be technically "correct" or "legal" to inject the ResilientWeatherService as the dependency to itself since the only requirement is that it implements IWeatherService, which of course would end up in rather strange behavior. So what ResilientWeatherService actually requires to function properly is a "real" WeatherService, and that's not really evident from the code and neither is what is a "real" WeatherService unless you dig into the implementations. Seems bit more of a clever than a good solution.
That's a great use case for goto, I'd never thought of it. I've always used recursion with a counter variable. Nice
Nice, thanks for video!
I am looking for this, but then a "genric" variation, e.g. a class that retries all functions of an interface. Not sure how to do this tough, maybe some proxy class/castle core can do this.
Decorator also known as Wrapper. Once again a thing that I used in the past without knowing the name.
This is not quite correct. "Wrapper" is a more generic term used for different patterns, but most of the time it is used as a synonym for the adapter pattern. And: Decorator and adapter are two different patterns. The decorator exposes the same interface as the inner service, the adapter exposes a different interface.
Hey Nick, enjoyed your talk and our chat at NDC today, keep it up with the great content!
I think it is similar with @Inject("key") in Angular
I don’t think this is good practice. Decorators shouldn’t care about the order, by doing this you’re adding knowledge of the order in which they’re used i.e. the resilient service always wants the “og”? What if you want to add a second third fourth decorator? Improper use of the keyed services imo.
What if you wanted to change the order I.e. before you had logging > resilient > og and you want to swap logging and resilient over? Then you’re violating the open/closed principle that you originally made the decorators to solve by editing class code, instead of simply changing your composition in the app root.
I don't like this approach. In the end, it doesn't save a registration to the DI container, and ends up coupling the decorator to a specific instance through a magic string. High coupling without type integrity seems to me like the worst of both worlds.
I find that the FromKeyedServicesAttribute to be an easy trap to fall into if you're just going to use it as-is in your code. That being said, I can see more interesting uses for it with codegen libraries that would produce proxies, such as an AOP framework.
it seems you can just auto generate the key instead of having it be set manually. you can have multiple bases this way.
But then you have to reference that key in the class that needs that implementation, and you have to do that in a static way. So you can't auto-generate that key. You might as well not use keys at all then, and create an extension method that wraps the original key-less code, I guess.
Thanks for explaining this to us Nick but it's more complexity for no value...
Looks like a named beans mess in Java. But at least my method "dOeS nOt Do MuLtIpLe ThInGs" so Uncle Bob can be happy.
I call bs. Using the key locks you into that key, can't swap out the `OpenWeatherService` against the `WewtherComWewtherService` without modifying the `ResiliantWeatherService`. This impl is basically a 180° rotation back to the start but a bit cleaner.
The `IWeatherService` is for consumption. The `IRawWeatherService` is for the API call. `IBaseWeatherService` has all the members. Both `IWeatherService` and `IRawWeatherService` inherit from `IBaseWeatherService`.
The `ResilientWeatherService` implements `IWeatherService` and takes `IEnumerable` via DI.
The resilient service can now do
1. Retying
2. Chain if responsibility
3. ...
This is one of the few cases where we throw multiple interfaces into one file.
Annotate the `IBase*` with some do not use directly warning or mark as obsolete, and suppress in in the other interfaces.
Seems familiar 😜
is there somebody like Nick Chapsas but teaching python?
I heard Chip Nacksas does Python.
I can see this getting confusing during a code review, and I would reject this if I saw it come through for anything that wasn't a wrapper for something we didn't have access to. Even then, I'm going to demand that either the DI be set up in a more readable way, or the service naming be clearer that it's a wrapper, and not run the risk of accidentally injecting itself infinitely if the DI setup is slightly wrong. This falls under "clever code" and should be rejected. Clever might not be bad, but if you come back to it 6 months later, are you going to know exactly what's going on at first glance? The video had to explain it, and it still doesn't "add behavior" to existing classes, so much as wraps them. "Adding behavior" is an extension method, not a wrapper.
I might be arguing semantics here, but spending as much time as I do fighting with code that was written like this over the course of years has given me a serious distaste for clever code that conceals something in some way. And the way the injection is set up here fits the bill perfectly. If you're wrapping a concrete implementation, inject the concrete implementation. Make the consumers of that implementation use the interface. Be explicit about it.
Why do MS make things so obscure. I much prefer AutoFac’s RegisterDecorator approach. The method gives clear meaning and you don’t need to specify a f’ing key.
Every comp. science students should watch Nick's vides imo. You're not going to learn programming, instead improve your vision. As Cse students the least thing we do is coding at school, we learn a lot of theory etc. And thats the youtube channel where you how to bind theory into code. I'm recommended your videos not only for this kinda videos but low-level understanding videos like the ones with Spans etc. People like Nick, sees a new feature, thinks differently, and be like "What if I use it for this also?" and this kind of vision-improving videos comes. Thank you master :)
What about scoped services? Is it required to configure it as singleton?
Why do you use Postman for the simplest cases while you have HTTP Client right in Rider?
Another use of middleware chain pattern and another ache for the true AOP support at language syntax level.
No! This does not allow you to decorate further, it's now locked. Say you wanted an analytics decorator between the decorator and the decoratee? You could not do that without modifying the decoratee. Also, the decoratee should not know it's being decorated.
OG, okay I know exactly what you want to say🤣
Not the biggest fan of this, if I'm honest...
goto... welcome to the 70s!
I had to check to see if I somehow time-skipped to April Fool's Day.
Another great content ❤❤
Extension everything
+1 for the proper use of goto.
In this case goto is not evil at all make more sense than a loop.
Nope, this is just a service collection trickery
wow, it went from 21.13c to 27.35c in minutes 😛
GlObAl bOiLinG!1!
I don't like this approach. It seems to be distributing the knowledge over what needs injected into multiple places because now the decorator service needs to know the attribute and the key to use for the injected service, in addition to having this all still controlled at the service registration point. IMO it's just better to stick to the service registration and using a factory there. It's cleaner and keeps the knowledge of how the service decorator is set up in one place.
Interesting concept, but far too much of a kludgy hack for my personal taste.
5:49 this doesn't look weird, this looks exactly like how this should be done
@@pyce. I'm not talking about the Goto, I'm talking about the pattern implementation
You are using labels to jump in the code? why not just put while or do while?
Would be cool to have services.AddSingleton().DecorateWith().DecorateWith().DecorateWith();
This is just wrong. By using key-based injection attribute in a service You clearly broke inversion of control paradigm in a way that the service now is aware of DI logic and you made it coupled to DI. Smells like an evident anti pattern. The service should not give a shit about injected dependecies creation details and DI mechanics related to that.
Well no, not going to use that
"GoTo"? OMG
LOL, "og"...'original gan...service'
Kuch nahi samjha 😢
very bad code
original what?? 😂
If you keep extending and extending isn't that going to have other developers get more confused new Developers