kicad-developers team mailing list archive
-
kicad-developers team
-
Mailing list archive
-
Message #18961
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