[Freeciv-Dev] Re: (PR#7229) Re: Re: Freeciv commit: ali: Add macro is_city_hilited().

["Jason Short" <jshort@xxxxxxxxxxxxxx>]

<URL: http://rt.freeciv.org/Ticket/Display.html?id=7229 >


> Although I've become used to it from Freeciv hacking, I would
> previously never have written something like this:
>
> {
>   if (same_pos(punit->x,
>                punit->y,
>                get_unit_in_focus()->x,    /* L1 */
>                get_unit_in_focus()->y)) {
>     ...
>   }
> }
>
> Instead I would've written:
> {
>   struct unit *focusunit = get_unit_in_focus();
>   if (same_pos(punit->x,
>                punit->y,
>                focusunit->x,    /* L1 */
>                focusunit->y)) {
>     ...
>   }
> }
>
> Do you expect a piece of silicon to generate an interrupt precisely
> at L1 which may change punit_focus via a GTK timer?

This is wrong on many levels.

1.  If the value ever were changed at L1, the first piece of code would be
wrong since it would use one unit's X value and another unit's Y value.

2.  Freeciv clients are single-threaded, so there will never be an
interrupt to change the value at L1.

3.  The second form is faster, but negligibly so (except in code that
"needs" optimization).  So it's really just a matter of style.  I much
prefer the second form.

jaosn