You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@rocketmq.apache.org by GitBox <gi...@apache.org> on 2019/12/27 01:38:27 UTC
[GitHub] [rocketmq] xiangwangcheng commented on a change in pull request
#1669: clean code
xiangwangcheng commented on a change in pull request #1669: clean code
URL: https://github.com/apache/rocketmq/pull/1669#discussion_r361560571
##########
File path: broker/src/main/java/org/apache/rocketmq/broker/client/ProducerManager.java
##########
@@ -206,35 +206,31 @@ public void unregisterProducer(final String group, final ClientChannelInfo clien
}
public Channel getAvaliableChannel(String groupId) {
- HashMap<Channel, ClientChannelInfo> channelClientChannelInfoHashMap = groupChannelTable.get(groupId);
- List<Channel> channelList = new ArrayList<Channel>();
- if (channelClientChannelInfoHashMap != null) {
- for (Channel channel : channelClientChannelInfoHashMap.keySet()) {
- channelList.add(channel);
- }
- int size = channelList.size();
- if (0 == size) {
- log.warn("Channel list is empty. groupId={}", groupId);
- return null;
- }
+ Channel checkChannel = null;
+ List<Channel> channelList = null;
- int index = positiveAtomicCounter.incrementAndGet() % size;
- Channel channel = channelList.get(index);
- int count = 0;
- boolean isOk = channel.isActive() && channel.isWritable();
- while (count++ < GET_AVALIABLE_CHANNEL_RETRY_COUNT) {
- if (isOk) {
- return channel;
- }
- index = (++index) % size;
- channel = channelList.get(index);
- isOk = channel.isActive() && channel.isWritable();
- }
- } else {
+ HashMap<Channel, ClientChannelInfo> channelClientChannelInfoHashMap = groupChannelTable.get(groupId);
+ if (null == channelClientChannelInfoHashMap || channelClientChannelInfoHashMap.isEmpty()) {
log.warn("Check transaction failed, channel table is empty. groupId={}", groupId);
- return null;
+ return checkChannel;
}
- return null;
+
+ channelList = new ArrayList<Channel>(channelClientChannelInfoHashMap.keySet());
+ int index = positiveAtomicCounter.incrementAndGet() % channelList.size();
+ Channel channel = null;
+ boolean isOk = false;
+ int count = 0;
+ do {
+ channel = channelList.get(index);
+ isOk = channel.isActive() && channel.isWritable();
+ if (isOk) {
+ checkChannel = channel;
Review comment:
`checkCannel` and `channel` local variable seem a little bit redundant. Is it cleaner to use one?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services