swarm/src: Remove ConnectionHandler #2519
Conversation
The `ConnectionHandler` trait is not exposed to users. The only implementor of `ConnectionHandler` is `NodeHandlerWrapper`. Thus `ConnectionHandler` is a superfluous abstraction. This commit removes `ConnectionHandler`. Next to this large change, this commit removes the `Tmuxer` trait parameter. `Swarm` enforces dynamic dispatching via `StreamMuxerBox` anyways, thus the trait parameter is useless. As a follow up to this commit one could rename `ProtocolsHandler` to `ConnectionHandler` and `NodeHandlerWrapper` to `ConnectionHandlerWrapper` or just `Wrapper`.
| /// Handler that processes substreams. | ||
| handler: THandler, | ||
| handler: NodeHandlerWrapper<THandler>, |
There was a problem hiding this comment.
Could we inline this struct into Connection now? Then we could just get rid of NodeHandlerWrapper completely!
I think the only reason we are passing this around at the moment is because we are constructing it "too early". We could just pass THandler around everywhere have all other fields of NodeHandlerWrapper be in Connection.
There was a problem hiding this comment.
I think the only reason we are passing this around at the moment is because we are constructing it "too early".
I think you are confusing the concept of IntoProtocolsHandler -> ProtocolsHandler, NodeHandlerWrapperBuilder -> NodeHandlerWrapper and now obsolete IntoConnectionHandler -> ConnectionHandler with the relation between ProtocolsHandler and NodeHandlerWrapper, right?
(Puuuh, so many XXXHandler. I hope we end up with only ProtocolsHandler (renamed to ConnectionHandler) once all the refactorings are in.)
Could we inline this struct into
Connectionnow?
I think so. At least move the two closer together. I suggest we rename ProtocolsHandler to ConnectionHandler first ( including some smaller clean-ups) and then consider folding NodeHandlerWrapper into Connection.
There was a problem hiding this comment.
I may confuse some things yes 😁
Either order is fine with me, it was a more general remark, not suggesting it should be done next or at all! :)
Same here :) |
The
ConnectionHandlertrait is not exposed to users. The onlyimplementor of
ConnectionHandlerisNodeHandlerWrapper. ThusConnectionHandleris a superfluous abstraction. This commit removesConnectionHandler.Next to this large change, this commit removes the
Tmuxertraitparameter.
Swarmenforces dynamic dispatching viaStreamMuxerBoxanyways, thus the trait parameter is useless.
As a follow up to this commit one could rename
ProtocolsHandlertoConnectionHandlerandNodeHandlerWrappertoConnectionHandlerWrapperor justWrapper.Follow up to #2492.