[GitHub] [rocketmq] Mr-Vincent opened a new issue #3917: [OPTIMIZATION] Class AsyncNettyRequestProcessor may be removed

[GitBox <gi...@apache.org>]

Mr-Vincent opened a new issue #3917:
URL: https://github.com/apache/rocketmq/issues/3917


   Those are code snippets in  method `org.apache.rocketmq.remoting.netty.AsyncNettyRequestProcessor#processRequestCommand` whitch is in module `remoting` :
   ```
   // 用于处理发送过来的请求
   if (pair.getObject1() instanceof AsyncNettyRequestProcessor) {
       // 这里异步 针对大多processor而言其实都是走同步（使用父类的默认逻辑）默认逻辑也是同步调用 和else里逻辑一样
       // 针对某些processor 会真正的异步 取决于processor会不会去重写async方法
       AsyncNettyRequestProcessor processor = (AsyncNettyRequestProcessor)pair.getObject1();
       processor.asyncProcessRequest(ctx, cmd, callback);
       log.error("当前req processor 为AsyncNettyRequestProcessor");
   } else {
       // 这里的逻辑是为了拓展，因为目前所有的processor都继承AsyncNettyRequestProcessor
       // 如果processor不去继承AsyncNettyRequestProcessor 那么这段逻辑就会触发 
       NettyRequestProcessor processor = pair.getObject1();
       // name server 中使用DefaultRequestProcessor 实例进行处理 属于业务层的实现，因此这里采用接口形式，便于不同业务去适配
       RemotingCommand response = processor.processRequest(ctx, cmd);
       callback.callback(response);
       log.error("当前req processor 不为AsyncNettyRequestProcessor");
   }
   ```
   I found that almost all implementations of `NettyRequestProcessor` had been extended `AsyncNettyRequestProcessor`. And the class `AsyncNettyRequestProcessor` just has one method:
   ```
   public abstract class AsyncNettyRequestProcessor implements NettyRequestProcessor {
   
       public void asyncProcessRequest(ChannelHandlerContext ctx, RemotingCommand request, RemotingResponseCallback responseCallback) throws Exception {
           RemotingCommand response = processRequest(ctx, request);
           responseCallback.callback(response);
       }
   }
   ```
   **So I had a question: why not move the only one method `asyncProcessRequest` to the interface 'NettyRequestProcessor'?** 
   In JDK8 we can define `default implementation` in interface just like this:
   ```
   public interface NettyRequestProcessor {
       RemotingCommand processRequest(ChannelHandlerContext ctx, RemotingCommand request)
           throws Exception;
   
       boolean rejectRequest();
   
       default void asyncProcessRequest(ChannelHandlerContext ctx, RemotingCommand request, RemotingResponseCallback responseCallback) throws Exception {
           RemotingCommand response = processRequest(ctx, request);
           responseCallback.callback(response);
       }
   
   }
   
   ```
   Then the class `AsyncNettyRequestProcessor ` can be removed and the code snippets in  `org.apache.rocketmq.remoting.netty.AsyncNettyRequestProcessor#processRequestCommand`  I mentioned above also can be simplified.Just remove the `if` judgment.
   
   For other `processors` which are implementations of `NettyRequestProcessor`,it's much easier to understand that we just implement interface 'NettyRequestProcessor'.


-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org