sashapolo commented on a change in pull request #153:
URL: https://github.com/apache/ignite-3/pull/153#discussion_r643855799
##########
File path:
modules/network/src/main/java/org/apache/ignite/network/internal/recovery/RecoveryClientHandshakeManager.java
##########
@@ -44,23 +44,29 @@
/** Handshake completion future. */
private final CompletableFuture<NettySender> handshakeCompleteFuture = new
CompletableFuture<>();
+ /** Message factory. */
+ private final NetworkMessagesFactory messageFactory;
+
/**
* Constructor.
*
* @param launchId Launch id.
* @param consistentId Consistent id.
*/
- public RecoveryClientHandshakeManager(UUID launchId, String consistentId) {
+ public RecoveryClientHandshakeManager(
+ UUID launchId, String consistentId, NetworkMessagesFactory
messageFactory
+ ) {
this.launchId = launchId;
this.consistentId = consistentId;
+ this.messageFactory = messageFactory;
}
/** {@inheritDoc} */
@Override public HandshakeAction onMessage(Channel channel, NetworkMessage
message) {
if (message instanceof HandshakeStartMessage) {
HandshakeStartMessage msg = (HandshakeStartMessage) message;
- HandshakeStartResponseMessage response =
HandshakeMessageFactory.handshakeStartResponseMessage()
+ HandshakeStartResponseMessage response =
messageFactory.handshakeStartResponseMessage()
Review comment:
I strongly disagree with this approach. Having static factories is
similar to having singletons, not a very appreciated design. Having factories,
that are injected, is a more flexible approach while being not very verbose
compared to the static factories.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
