The Smart Way of Using the Decorator Pattern in C#
Вставка
- Опубліковано 30 сер 2023
- Use code DDD20 and get 20% off the brand new Domain-Driven Design course on Dometrain: dometrain.com/course/getting-...
Get the source code: mailchi.mp/dometrain/z8d6n6wtuk
Become a Patreon and get special perks: / nickchapsas
Hello, everybody, I'm Nick, and in this video, I will show you how you can change your code's behaviour without changing the code directly. We will be using a technique called Decoration and we will also use a new .NET 8 feature to make it even more interesting.
Subscribe to Amichai: @amantinband
Workshops: bit.ly/nickworkshops
Don't forget to comment, like and subscribe :)
Social Media:
Follow me on GitHub: bit.ly/ChapsasGitHub
Follow me on Twitter: bit.ly/ChapsasTwitter
Connect on LinkedIn: bit.ly/ChapsasLinkedIn
Keep coding merch: keepcoding.shop
#csharp #dotnet
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.
This is a bit like Inception, except you never go more than 1 level. Any deeper is a path to madness...
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
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.
This was very cool. I always learn something. I will try this out today. Thanks Nick.
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.
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 ;)
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# 😅
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
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
Nice, thanks for video!
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
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 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 😂
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 :)
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
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.
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.
Used to have decorators done but was using Autofac. It was really simple
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.
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.
This is interesting for extending sealed classes for other things i would probably just use inheritance
i was hoping for something more general. this is specific to Dependency Injection, where you can count on the interface already existing. i think we need language support for the decorator pattern because it sucks having to link up all the methods etc (this multiplies the work depending on how many there are). for inheritance we already have language support in the form "class A : B" we need something similar but for the decorator pattern and it would be nice to be able to somehow pick and choose which methods etc. we want to use from the existing class. my gut feeling is that there is a deeper underlying problem in the type system. i like your creativity though
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.
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. 🙂
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.
9:10 needs to receive more love! :D
Nick you are og, i love it.
How do you feel about using Scrutor for decorating instead?
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?
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.
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 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
I don't like adding the FromKeyedService attribute to the implementation class, because now this class is dependent on a DI container.
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#
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.
Hey Nick, enjoyed your talk and our chat at NDC today, keep it up with the great content!
What about extension methods Nick , they also can modify behavior of a class without touching it ?
That's a great use case for goto, I'd never thought of it. I've always used recursion with a counter variable. Nice
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.
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
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
I think it is similar with @Inject("key") in Angular
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?
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'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....
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.
Oh yes! Decorating pattern holy wars in comments!
The goto made me cry :)
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.
I think we can use scrutor nuget package to decorate in a simpler way
Another great content ❤❤
Really don't like the key based approach, mainly because of the introduction of a string and the dependency on the attribute.
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();
}
}
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.
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...
OG, okay I know exactly what you want to say🤣
Extension everything
What about scoped services? Is it required to configure it as singleton?
Seems familiar 😜
is there somebody like Nick Chapsas but teaching python?
I heard Chip Nacksas does Python.
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 :)
Another use of middleware chain pattern and another ache for the true AOP support at language syntax level.
Looks like a named beans mess in Java. But at least my method "dOeS nOt Do MuLtIpLe ThInGs" so Uncle Bob can be happy.
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 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.
goto... welcome to the 70s!
I had to check to see if I somehow time-skipped to April Fool's Day.
Why do you use Postman for the simplest cases while you have HTTP Client right in Rider?
I understand that you try to show the concept @nickchapsas and you even note that one should do it their own way, but I just don't agree that this is the right way to demonstrate anything at all. It opens the gate for some inexperienced developers to take away the wrong message and produce unnecessary, convoluted code. 😢
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.
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.
wow, it went from 21.13c to 27.35c in minutes 😛
GlObAl bOiLinG!1!
Nope, this is just a service collection trickery
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.
+1 for the proper use of goto.
In this case goto is not evil at all make more sense than a loop.
5:49 this doesn't look weird, this looks exactly like how this should be done
you can't be serious
@@pyce. I'm not talking about the Goto, I'm talking about the pattern implementation
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.
Interesting concept, but far too much of a kludgy hack for my personal taste.
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.
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();
"GoTo"? OMG
Well no, not going to use that
Not the biggest fan of this, if I'm honest...
LOL, "og"...'original gan...service'
original what?? 😂
Kuch nahi samjha 😢
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.
very bad code
If you keep extending and extending isn't that going to have other developers get more confused new Developers