[ 
https://issues.apache.org/jira/browse/KAFKA-1910?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14491698#comment-14491698
 ] 

Jay Kreps commented on KAFKA-1910:
----------------------------------

Hey [~guozhang], on the whole this made things a lot better, but a couple 
things seemed to get worse from my opinion. 

1. Coordinator has a lot of methods named initiateXyzRequest that no longer 
actually initiate the request after this refactoring. One of my pet peeves is 
methods that say they do something they don't do. The request initiation has 
been moved into sendAndReceive. These initiateXyz methods are a bit awkward 
because they aren't really createXyzRequest either as they do a bunch of 
stateful prep. I think this would make more sense if initiateXyzRequest did the 
send and sendAndReceive became something like completeRequest and just 
completed an already initiated request (or whatever).

2. You moved the completion handlers into their own objects. I started that way 
but one thing I found was that in practice since each handler should have one 
method that invokes that request separating these into other objects really 
breaks up the control flow (often the completion handler is doing a bunch of 
things very tightly tied to what happens during the request setup. I actually 
think the prior approach where the handler was defined as an annonymous 
callback in the method that does the setup was clearer.

These are both somewhat matters of individual taste, and like I said almost 
everything got better, but I thought I would pass on the feedback.

> Refactor KafkaConsumer
> ----------------------
>
>                 Key: KAFKA-1910
>                 URL: https://issues.apache.org/jira/browse/KAFKA-1910
>             Project: Kafka
>          Issue Type: Sub-task
>          Components: consumer
>            Reporter: Guozhang Wang
>            Assignee: Guozhang Wang
>             Fix For: 0.8.3
>
>         Attachments: KAFKA-1910.patch, KAFKA-1910.patch, KAFKA-1910.patch, 
> KAFKA-1910_2015-03-05_14:55:33.patch
>
>
> KafkaConsumer now contains all the logic on the consumer side, making it a 
> very huge class file, better re-factoring it to have multiple layers on top 
> of KafkaClient.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to