discordgo
discordgo copied to clipboard
bwmarrin
proposal: update handler function signatures to use an interface for the session
I've started to use Discord Go to write a bot, and have found it somewhat difficult to properly test without writing my own shimmed handlers.
This challenge exists because the first argument to handlers is a struct type and not an interface, so a mock implementation can't be provided to assert proper behaviors. The standard library has solved for this exact problem as the first argument to a handler (http.ResponseWriter) is an interface, so that alternative implementations can be provided for testing.
I'd like to propose a breaking change to the library to update the handler signature to go from discordgo.Session being a struct to it becoming an interface. So from:
func handler(s *discordgo.Session, <eventType>) {}
to
func handler(s discordgo.Session, <eventType>) {}
The benefit of making this change is that Discord Go handler functions / methods could be tested directly, without needing to write shims for each handler you implement. This would make it much easier to test, and the ease of which could be shown-off in the example projects. It also may not be a major breaking change for some users, if they only use the methods on the type, which would be a good benefit.
One downside of this change is that the size of the *discordgo.Session method set is large today. Even using composition, a discordgo.Session interface would end up becoming quite large. We would also need to add more methods, getters and setters, to tweak the internal settings.
Overall, I think this would be a net-win for the project and would like to encourage its inclusion in the 0.20.0 development cycle.
Could we not stay with a pointer but use an interface instead of the raw structure ?
@Hinara I'm not sure I completely understand, are you asking about changing the discordgo.Session type to be an interface instead of a struct, but keep it as a pointer value?
Assuming I am understanding that correctly, you pretty much never want something to be a pointer to an interface unless you plan on swapping out the thing that's boxed inside of that interface. We don't plan on doing that inside of discordgo, I don't think, so I would assume we don't want it. But also if you're using an interface value, it should not be a pointer type since consumers will need to manually dereference it before invoking any methods:
- https://goplay.space/#DXs9uIlCfhG (notice the compile time errors)
To summarize, we should change the type from struct to interface and make it no longer a pointer. Keeping it as a pointer would make it much more of a breaking change.
hmm ok I didn't remembered the way golang work with intefaces ^^' Elsewhere I thing this can be a really cool feature :)