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

ASF GitHub Bot commented on ROCKETMQ-96:
----------------------------------------

Github user Jaskey commented on the issue:

    https://github.com/apache/incubator-rocketmq/pull/60
  
    @shroman @lizhanhui @zhouxinyu 
    
    There are users asking the same question about the logic of consume orderly 
and they both complain that the `msgTreeMapTemp` is very hard to understand 
it's implication, so would you please review this pr and merge it in 4.1.
    
    Besides, I wonder if there is the same issue in below method, the signature 
of the method is trying to determine if there are some tmp msg(which will be 
only used in consumer orderly service), while the implementation is to 
determine the `msgTreeMap`?
    
    Is the implementation wrong or the is signature misleading.
    
        public boolean hasTempMessage() {
            try {
                this.lockTreeMap.readLock().lockInterruptibly();
                try {
                    return !this.msgTreeMap.isEmpty();
                } finally {
                    this.lockTreeMap.readLock().unlock();
                }
            } catch (InterruptedException e) {
            }
    
            return true;
        }


> Rename tmp variable 
> --------------------
>
>                 Key: ROCKETMQ-96
>                 URL: https://issues.apache.org/jira/browse/ROCKETMQ-96
>             Project: Apache RocketMQ
>          Issue Type: Improvement
>          Components: rocketmq-client
>    Affects Versions: 4.0.0-incubating
>            Reporter: Jaskey Lam
>            Assignee: Jaskey Lam
>            Priority: Minor
>             Fix For: 4.1.0-incubating
>
>
> Some variable is named with tmp which is very difficult to read and 
> understand its usage.
> For exampe:
> 1. in sendDefaultImpl, when select message queue, the variable is named with 
>     
>       MessageQueue tmpmq = this.selectOneMessageQueue(topicPublishInfo, 
> lastBrokerName);
> 2. In process queue, a field is named `msgTreeMapTemp`, which takes me 
> minutes to understand why it is desgined for, which should be rename to some 
> name like consumeingOrderlyMsgTreeMap



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

Reply via email to