This repository has been archived by the owner on Jun 30, 2023. It is now read-only.
Simplify behavior execution delegate signature #79
Labels
enhancement
New feature or request
Comments
kzu
added a commit
that referenced
this issue
Jan 16, 2021
Chain of responsibility pattern implementation has a few variants, but it seems to be more common for the "next" delegate/handler to actually be a direct reference to the next handler in the chain, so you can execute it directly. This simplifies the calling pattern and signature since you change from: next().Invoke(...) to just next.Invoke(...) which is far more natural. Moving the "getting next" (or throwing if not valid) to be part of the invocation of the delegate itself, simplifies the consumers of the pipeline at a just a little expense on the behavior pipeline side. This is a net plus IMO. In addition, it's explicitly advised against to append Delegate to delegate names, so we should instead rename consistently to ***Handler instead (ExecuteHandler and AppliesToHandler). We'd have one less delegate since the GetNextBehavior would go away. This is a breaking change in the public API, so we update that file accordingly. This is the time to do this before v1 ships to stable. Fixes #79
kzu
added a commit
that referenced
this issue
Jan 16, 2021
Chain of responsibility pattern implementation has a few variants, but it seems to be more common for the "next" delegate/handler to actually be a direct reference to the next handler in the chain, so you can execute it directly. This simplifies the calling pattern and signature since you change from: next().Invoke(...) to just next.Invoke(...) which is far more natural. Moving the "getting next" (or throwing if not valid) to be part of the invocation of the delegate itself, simplifies the consumers of the pipeline at a just a little expense on the behavior pipeline side. This is a net plus IMO. In addition, it's explicitly advised against to append Delegate to delegate names, so we should instead rename consistently to ***Handler instead (ExecuteHandler and AppliesToHandler). We'd have one less delegate since the GetNextBehavior would go away. This is a breaking change in the public API, so we update that file accordingly. This is the time to do this before v1 ships to stable. Fixes #79
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Chain of responsibility pattern implementation has a few variants, but it seems to be more common for the "next" delegate/handler to actually be a direct reference to the next handler in the chain, so you can execute it directly. This simplifies the calling pattern and signature since you change from:
next().Invoke(...)to justnext.Invoke(...)which is far more natural.Moving the "getting next" (or throwing if not valid) to be part of the invocation of the delegate itself, simplifies the consumers of the pipeline at a just a little expense on the behavior pipeline side. This is a net plus IMO.
In addition, it's explicitly advised against to append
Delegateto delegate names, so we should instead rename consistently to***Handlerinstead (ExecuteHandlerandAppliesToHandler). We'd have one less delegate since theGetNextBehaviorwould go away.The text was updated successfully, but these errors were encountered: