← Back to team overview

kicad-developers team mailing list archive

Re: [PATCH] Cleanup: remove unnecessary macros

 

Le 26/06/2015 14:17, Chris Pavlina a écrit :
> On Fri, Jun 26, 2015 at 08:45:08AM +0200, jp charras wrote:
>> I am *not* convinced by replacing
>>> m_pos.x -= aYaxis_position;
>>> NEGATE(  m_pos.x );
>>> m_pos.x += aYaxis_position;
>>
>> by
>>> m_pos.x = 2 * aYaxis_position - m_pos.x;
>>
>> When you read the last form, it is not clear this is a mirroring
>> relative to the aYaxis_position X position.
>>
>> At least for me, it is not "a lot clearer...", this is quite the opposite.
>>
>> Perhaps a macro like MIRROR( m_pos.x, aYaxis_position )
>> is "a lot clearer..."
> 
> Yup, that's fair. I still think that "m_pos.x = 2 *....." is clearer 
> than the original, but that's a matter of opinion, and I _do_ like 
> MIRROR(). That's even clearer still. Done - modified patch attached.
> 
>>
>> By the way, for me:
>> "x = -x" looks better than "x *= -1"
>> an should be faster to calculate.
> 
> Also done. "x = -x" is more technically correct, so I can appreciate 
> that (and in the case of C++ objects, will call the correct operator - 
> unary negation instead of multiplication).
> 
> To nitpick a bit, it's NOT faster to calculate. gcc compiles "x *= -1" 
> to a "negl" instruction just like "x = -x" even on the lowest 
> optimization setting; clang does not do this optimization on -O0 but 
> does it on all higher settings. (And even if they didn't perform this 
> optimization, the difference is _incredibly_ negligible compared to 
> everything else being done in the vicinity.)
> 
> --
> Chris

Committed. Thanks.

-- 
Jean-Pierre CHARRAS


References