[UPDATE: I have since updated this post. Look for the block at the bottom]\
The Problem
Anoop’s Chain Of Responsibility example starts with this://Request class LoanRequest { public string Customer { get; set; } public decimal Amount { get; set; } } //Abstract Request Handler interface IRequestHandler { string Name { get; set; } void HandleRequest(LoanRequest req); IRequestHandler Successor { get; set; } }[Insert record-scratch sound here]
I couldn’t get past this part without writing this blog post.
Can you see the problem? … No? Let me give you a hint:
//Abstract Request Handler interface IRequestHandler { string Name { get; set; } void HandleRequest(LoanRequest req); ------======>>> ------======>>> IRequestHandler Successor { get; set; } ------======>>> }WAIT! Hold on! Why does the IRequestHandler know about its successor? Why would I, as a consumer of an IRequestHandler want access to its successor? To me, there is no part of IRequestHandler’s responsibility that involves knowing about and/or forwarding on a request to some other instance of itself.
The Solution
I prefer more of a MVA/DMV mentality for the IRequestHandler(“It’s not MY Job, go somewhere else!”)
// New Request Handler interface IRequestHandler { string Name { get; set; } bool HandleRequest(LoanRequest req); }In this approach, the HandleRequest method returns a boolean telling whether or not it actually handled the request.
And Another Thing
One other flaw of Anoop’s approach: if the request makes it all the way through the chain and doesn’t get handled, we have no indication that it was not handled. In the new approach, we get a nice, pleasant boolean value.What Am I Forgetting?
Wait, you say? What about the Chain Of Responsibility? I don’t see any Chain and you promised me a Chain!Oh …. yeah… ok..
So, now that the interface makes me feel all warm and fuzzy, we need to implement the Chain Of Responsibility pattern. I prefer to implement the Chain Of Responsibility in terms of a variation on the Composite Pattern:
class ChainingRequestHandler : IRequestHandler { private readonly IEnumerable<IRequestHandler> _handlers; public ChainingRequestHandler(IEnumerable<IRequestHandler> handlers) { _handlers = handlers; } public string Name { get; set; } public bool HandleRequest(LoanRequest req) { // iterate of the chain foreach(IRequestHandler handler in _handlers) { if (handler.HandleRequest(req) == true) { // it was handled return true; } } // no such luck return false; } }
Conclusion
By using the a variation on the composite pattern, we allow the client code to use the nice, simple IRequestHandler without any knowledge of the underlying complexity of implementation, or that there’s even a Chain Of Responsibility at all.I don’t know about you, but that warms the cockles of my development heart.
Now, doesn’t that feel better?
Now, I think I should go read the rest of that article!
[UPDATE]
So, after I blogged about it and whined on Twitter, I finished the rest of the article, which was a nice application of MEF to the pattern. I also read some other MEF-related posts on Anoop's blog, which were also informative. I exchanged a few tweets with Anoop as well.
I should point out that Anoop's implementation is actually exactly what the Chain Of Responsibility pattern is supposed to be (I'm suspecting the void return value was just a bug/oversight). My actual problem is with the Chain Of Responsibility pattern itself.
From a business perspective, I understand the idea of knowing the "successor" in the chain. In this instance, of course the Cashier knows who her Manager is at a given time, but:
- What happens when there's a shift change?
- What happens if there are two Managers in the office?
- What happens if there are zero Managers in the office?
I just feel that the stock pattern limits the implementation too much. I also believe that my approach, while handling the stock Chain Of Responsibility goals well enough also has added benefits.
First, It adds an implementation abstraction from the client code. The user of IRequestHandler does not need to know about Successors or anything else like that. In stead of having client code talk to a class that then talks to the Chain, the Chain can be handed back to the client code in the form of the simple interface.
Secondly, given this new abstraction, it simplifies the interfaces while allowing a greater abstraction for the Chaining. Thus, the developer can implement some sort of Factory/Bootstrapping/IoC implementation for creating the Handler used by the client code. This way, the Chain ordering can be redefined, the Chain can be replaced with a Directed Graph or a Workflow without the client code OR the IRequestHandler-derived classes being any the wiser.
So, my secondary conclusion here is: Anoop didn't do anything wrong, it's just that the Chain Of Responsibility pattern should really have 1 extra level of abstraction in it to make it as flexible as possible.
As a follow up, I did finish the article and I think it's nice and informative. MEF is new to me (it's been on my 'check this out' list for quite a long time though).
ReplyDeleteIn this particular example, I can see how a hierarchical arrangement like Bank employees would have a known "successor", but I feel that bank employees aren't generic "Request Handlers".
So, by pushing the knowledge into the Metadata attribute, he removes the knowledge of the successor from the RequestHandler which is much better :)
Not sure if you read my article completely.
ReplyDeleteThe actual CoR definition itself specifies there is a pointer to the successor http://www.oodesign.com/chain-of-responsibility-pattern.html
I was suggesting a variation, where the items in the chain are decoupled :)
Yes, you are correct. I plan on tweaking this post to kinda point that direction.
ReplyDeleteOur two end approaches accomplish almost the same thing (outside of MEF), by removing that successor knowledge from the class itself, though the Metadata attribute is *really* close to being coupled to the class. Does MEF support adding Metadata later?
This comment has been removed by the author.
ReplyDelete