[ 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)