serenity
serenity copied to clipboard
Remove `CacheAndHttp`, plus cleanup of dispatch
The CacheAndHttp struct has too similar a name to the CacheHttp trait, which might cause confusion. Therefore, I've removed the struct, and inlined its fields where it was used before. In addition, cleaned up some of client/dispatch.rs, for example the dispatch and handle_event functions now take a Context as parameter (to reduce cloning), and dispatch is now a regular async fn, rather than returning a BoxFuture. Also introduced an impl of Debug to Context so that the tracing instrumentation on handle_event remains unchanged.
Is it worth the churn? [referring to the "removing CacheAndHttp" part]
This PR also seems to be multi-purpose, and last time I tried cleaning up this code in a multi-purpose commit everything broke. I believe this should be split up into separate PRs (cleaning up dispatch, possibly removing cacheandhttp, etc)
I made my best attempt not to touch any of the caching logic, just simplifying and cleaning things up. I tested with some example bots and it seems to work fine, but you're welcome to clone the branch and test some more substantial code with it.
As for justification, these two fields were wrapped (I presume) to hide the feature-flag on the cache, in an attempt to please the compiler when building without cache enabled. But the struct itself is rather meaningless otherwise, and only adds mental overhead. Plus, the CacheHttp trait abstracts over pretty much the exact same thing, but is useful in more places.
For #2138, I was thinking of storing CurrentUser in CacheAndHttp as a third field. Could we keep CacheAndHttp for now to keep that possibility open?
Going to split this up into two separate PRs. Closing for now.