Re: /bzr/squid3/trunk/ r12194: Small optimization in CommOpener statistic accounting.

On 07/02/2012 04:34 PM, Amos Jeffries wrote:
> On 03.07.2012 03:30, Francesco Chemolli wrote:
>>
>> - if (conn_->getPeer())
>> - conn_->getPeer()->stats.conn_open++;
>> + if (peer *peer=(conn_->getPeer()))
>> + ++peer->stats.conn_open;
>>
>> lookupLocalAddress();
>
>
> Two points:
>
> 1) assignment in the if() needs to be double-bracketed around the whole
> = operator expression, not just the RHS:
> if ((peer *peer=conn_->getPeer()))

No, please do not do that! This is not a regular assignment, it is a
variable declaration with initialization. It does not need more
brackets. In fact, I would not be surprised if some compilers will fail
if those extra brackets are included.

The problem with that expression is that the variable name shadows the
type (it is usually not a bug due to C++ name resolution rules but bad
style).

It also uses too many brackets on the RHS. I know that some like extra
brackets (and then skimp on spaces, resulting in a really hard-to-read
stream of characters!) so I am not going to fight about this aspect.

The best syntax in this particular context is

    if (peer *p = conn_->getPeer())

HTH,

Alex.
Received on Tue Jul 03 2012 - 17:29:32 MDT