You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Is your feature request related to a problem? Please describe.
The current convention defines EventHandlers by naming them according to which event they handle, e.g. TodoItemCreatedEventHandler handles the TodoItemCreatedEvent. In the current codebase that handlers logs an informational line informing the reader that the event was handled.
This quickly becomes convoluted as singular events may have many consequences, this TodoItemCreatedEvent might for example need the currently implemented informational log, send a push notification to users, and send a SignalR message to other browser clients so they can refresh the Todo list to show the new item.
Implementing all these 3 things in a single class (the currently TodoItemCreatedEventHandler) would violate the single-responsibility principle and make it an unwieldy class with many dependencies (a push notification service, a signalR notification hub, and the logger) which would make it difficult to test this class as much mocking code would need to written in order to instantiate this class for tests.
On top of that, it would also introduce an additional problem in that the handler's actions are not transactional, for example the log and push notification might succeed, but the signalR message might fail to send. Buillding a retry behaviour around the handler would become more difficult, as the handler would need to save state (introducing yet another dependency!) about which of the several of its responsibilities have already completed successfully, as to not make multiple logs or send multiple push notifications about the same event.
Describe the solution you'd like
In the past I've agreed with teams to name handlers which affect the system (this excludes query handlers) to what they do, instead of which messages (messaged being either commands or events) they handle.
In this concrete example I would rename the current TodoItemCreatedEventHandler to LogTodoItemCreated, to make it clear that this class's only responsiblity is logging the created event.
Adding functionality about additional consequences the TodoItemCreatedEvent should have, would go into new handlers:
NotifyUserAboutNewTodoItem: Send push notification to user
RefreshTodoList: Send SignalR message to browser to refresh todo item list
Note that there can also be handlers which handle both events and commands, because they have the same consequence. For example we could have a DeleteTodoItem handler which can act on DeleteTodoItem commands, but also on TodoItemDueDatePassed events, to automatically remove todo items once their due date has passed (I agree this wouldn't seem like a good feature to have in a to-do application, but I have a hard time coming up with a better example while sticking to the 'todo' business logic)
From a file-tree perspective, this would change the layout of the Application folder like so:
ToDoItems/
Commands/
CreateTodoItemCommand.cs
... (no handlers)
Handlers/
CreateTodoItem.cs
LogTodoItemCreated.cs
LogTodoItemCompleted.cs
Describe alternatives you've considered
None
Additional context
N/A
The text was updated successfully, but these errors were encountered:
Is your feature request related to a problem? Please describe.
The current convention defines EventHandlers by naming them according to which event they handle, e.g.
TodoItemCreatedEventHandler
handles theTodoItemCreatedEvent
. In the current codebase that handlers logs an informational line informing the reader that the event was handled.This quickly becomes convoluted as singular events may have many consequences, this
TodoItemCreatedEvent
might for example need the currently implemented informational log, send a push notification to users, and send a SignalR message to other browser clients so they can refresh the Todo list to show the new item.Implementing all these 3 things in a single class (the currently
TodoItemCreatedEventHandler
) would violate the single-responsibility principle and make it an unwieldy class with many dependencies (a push notification service, a signalR notification hub, and the logger) which would make it difficult to test this class as much mocking code would need to written in order to instantiate this class for tests.On top of that, it would also introduce an additional problem in that the handler's actions are not transactional, for example the log and push notification might succeed, but the signalR message might fail to send. Buillding a retry behaviour around the handler would become more difficult, as the handler would need to save state (introducing yet another dependency!) about which of the several of its responsibilities have already completed successfully, as to not make multiple logs or send multiple push notifications about the same event.
Describe the solution you'd like
In the past I've agreed with teams to name handlers which affect the system (this excludes query handlers) to what they do, instead of which messages (messaged being either commands or events) they handle.
In this concrete example I would rename the current
TodoItemCreatedEventHandler
toLogTodoItemCreated
, to make it clear that this class's only responsiblity is logging the created event.Adding functionality about additional consequences the
TodoItemCreatedEvent
should have, would go into new handlers:NotifyUserAboutNewTodoItem
: Send push notification to userRefreshTodoList
: Send SignalR message to browser to refresh todo item listNote that there can also be handlers which handle both events and commands, because they have the same consequence. For example we could have a
DeleteTodoItem
handler which can act onDeleteTodoItem
commands, but also onTodoItemDueDatePassed
events, to automatically remove todo items once their due date has passed (I agree this wouldn't seem like a good feature to have in a to-do application, but I have a hard time coming up with a better example while sticking to the 'todo' business logic)From a file-tree perspective, this would change the layout of the
Application
folder like so:Describe alternatives you've considered
None
Additional context
N/A
The text was updated successfully, but these errors were encountered: