Github user pnowojski commented on a diff in the pull request:

    https://github.com/apache/flink/pull/4533#discussion_r144214324
  
    --- Diff: 
flink-runtime/src/main/java/org/apache/flink/runtime/io/network/netty/CreditBasedClientHandler.java
 ---
    @@ -272,4 +316,53 @@ private void decodeBufferOrEvent(RemoteInputChannel 
inputChannel, NettyMessage.B
                        bufferOrEvent.releaseBuffer();
                }
        }
    +
    +   private void writeAndFlushNextMessageIfPossible(Channel channel) {
    +           if (channelError.get() != null) {
    +                   return;
    +           }
    +
    +           if (channel.isWritable()) {
    --- End diff --
    
    Sorry, I could be more clear :) I meant that shorter of the if/else branch 
should go first for readability. Here instead of:
    ```
    if (channel.isWritable()) {
        // long
        // block
        {
            // of
            {
                // nested
                // code
            }
        }
    }
    ```
    you could have:
    ```
    if (!channel.isWritable()) {
        return;
    }
    // long
    // block
    {
        // of
        {
            // nested
            // code
        }
    }
    }
    ```
    which is easier to understand because reader immediately knows that one of 
the branch is super simple and code doesn't nest that deeply.


---

Reply via email to