As of now, event handling has been improved with the events subpackage, but actual usage is still not pretty.
Event hooks still have to deal with Event interface{} in DiscordEvent and then do type assertions and type casting (in case they are subscribed to multiple events). Needless to say, things like this are not idiomatic:
func onChatlogEvent(ctx *unison.Context, dv *events.DiscordEvent) (bool, error) {
var m *discordgo.Message
// Check event type
switch ev := dv.Event.(type) {
default:
return false, nil
case *discordgo.MessageCreate:
m = ev.Message
}
// ...
}
Instead, I want to propose creating interfaces for groups of related events.
If you look at the event/types.go, you see that there are a lot of events that are related to the same subject (MessageCreate/Update/Delete).
Most of these events in discordgo also only embed the underlying struct (e.g. struct MessageCreate { *Message }). Some discordgo events also don't contain a timestamp, which might be useful in cases where actual event processing is delayed.
To improve the event hook API and making the code more testable, I believe that we should:
- Rename current types in the
events package, so that FooEvent and every other exported type ending with Event will be an interface
- Create an interface for all discord event classes rather than exporting the current
DiscordEvent wrapper type
- Add timestamps to all events and make all event interfaces export an
Action() or Operation() method that reflects the nature of the event
Example code:
type discordEvent struct {
// current DiscordEvent code here
// ReceivedAt is the timestamp when the event was received
ReceivedAt time.Time
}
type DiscordEvent interface {
// Type returns the type identifier of the event
Type() EventType
// Event returns the discordgo event, which has to be typecasted for usage
Event() interface{}
}
// PersistenceAction is an enum reprenting the default actions covered by most event groups
type PersistenceAction int
const (
CreateAction PersistenceAction = iota
UpdateAction
DeleteAction
)
type MessageEvent interface {
DiscordEvent
Action() PersistenceAction
Message() *discordgo.Message
}
Note:
Type-specific events such as MessageEvent should probably still define their own Action type.
However, this will lead to problems comparing e.g. PersistenceAction values and MessageAction values.
Whether it's worth having an Action() method I am not entirely sure about yet, since the action is already clear from Type(). There might be benefit to having a Action() reside in DiscordEvent and define ALL possible actions in a single enum. Opinions please!
Now the EventHook type can be adjusted to contain callbacks for any class of events, and the Events list will be made obsolete, since EventDispatcher can now simply check if a callback exists:
// EventHook is an application-level type for handling discord requests.
// All callbacks are optional, and whether they are defined or not
// is used to determine whether the EventDispatcher will send events to them.
type EventHook struct {
// current EventHook fields here
// OnEvent is called for all events.
// Handlers must typecast the event type manually, and ensure
// that it can handle receiving the same event twice if a type-specific
// callback also exists.
OnEvent func(ctx *Context, ev events.DiscordEvent) error
// OnMessageEvent is called for every message-related event.
OnMessageEvent func(ctx *Context, ev events.MessageEvent) error
// OnConnectionEvent ...
// OnUserEvent ...
// OnChannelEvent ...
// OnGuildEvent ...
}
This will simplify most use-cases of event hooks, avoiding type assertions and type casting when not explicitely required.
The trade-off here is that EventHooks will be fired on actions they might not want to handle. I think the performance impact would be minimal however and worth the improvements to the packages API for users and testability.
As of now, event handling has been improved with the
eventssubpackage, but actual usage is still not pretty.Event hooks still have to deal with
Event interface{}inDiscordEventand then do type assertions and type casting (in case they are subscribed to multiple events). Needless to say, things like this are not idiomatic:Instead, I want to propose creating interfaces for groups of related events.
If you look at the event/types.go, you see that there are a lot of events that are related to the same subject (MessageCreate/Update/Delete).
Most of these events in discordgo also only embed the underlying struct (e.g.
struct MessageCreate { *Message }). Some discordgo events also don't contain a timestamp, which might be useful in cases where actual event processing is delayed.To improve the event hook API and making the code more testable, I believe that we should:
eventspackage, so thatFooEventand every other exported type ending withEventwill be an interfaceDiscordEventwrapper typeAction()orOperation()method that reflects the nature of the eventExample code:
Note:
Type-specific events such as
MessageEventshould probably still define their own Action type.However, this will lead to problems comparing e.g.
PersistenceActionvalues andMessageActionvalues.Whether it's worth having an
Action()method I am not entirely sure about yet, since the action is already clear fromType(). There might be benefit to having aAction()reside inDiscordEventand define ALL possible actions in a single enum. Opinions please!Now the
EventHooktype can be adjusted to contain callbacks for any class of events, and theEventslist will be made obsolete, sinceEventDispatchercan now simply check if a callback exists:This will simplify most use-cases of event hooks, avoiding type assertions and type casting when not explicitely required.
The trade-off here is that EventHooks will be fired on actions they might not want to handle. I think the performance impact would be minimal however and worth the improvements to the packages API for users and testability.