ivankelly commented on a change in pull request #1103: PIP-13-1/3: Provide 
`TopicsConsumer` to consume from several topics under same namespace
URL: https://github.com/apache/incubator-pulsar/pull/1103#discussion_r167516056
 
 

 ##########
 File path: 
pulsar-client/src/main/java/org/apache/pulsar/client/impl/UnAckedMessageTracker.java
 ##########
 @@ -18,26 +18,24 @@
  */
 package org.apache.pulsar.client.impl;
 
+import io.netty.util.Timeout;
+import io.netty.util.TimerTask;
 import java.io.Closeable;
-import java.util.ArrayList;
 import java.util.HashSet;
-import java.util.List;
 import java.util.Set;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.locks.Lock;
 import java.util.concurrent.locks.ReentrantReadWriteLock;
-
+import java.util.function.Predicate;
+import org.apache.pulsar.client.api.MessageId;
 import org.apache.pulsar.common.util.collections.ConcurrentOpenHashSet;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import io.netty.util.Timeout;
-import io.netty.util.TimerTask;
-
 public class UnAckedMessageTracker implements Closeable {
 
 Review comment:
   For both PartitionedConsumer and TopicsConsumer you know what kind of 
MessageId you will be need.
   For PartitionedConsumer you can create a 
UnAckedMessageTracker<MessageIdImpl>.
   For ConsumerConsumer you can create an UnAckedMessageTracker<MessageIdImpl>.
   For TopicsConsumer you can create an 
UnAckedMessageTracker<TopicMessageIdImpl>.
   
   Any cast, at all, is code smell. Looking at this again, you should also have 
a specialization of UnAckedMessageTracker<TopicMessageIdImpl>, and only this 
class should implement #removeTopicMessages.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

Reply via email to