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

["Arnstein Lindgard" <a-l@xxxxxxx>]

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

On Sat, 10 Jan 2004 12:52:26 -0800 Jason Short wrote:

> > {
> >   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.

Which is my point exactly. We are producing C code that accounts for
the possibility of an interrupt at L1, even though we know that it
will not modify punit_focus AND it would have been all bollocks if it
did. I see style 1 (excessive usage of function calls) as suitable if
we for some reason needed an updated focus unit (return value of the
function) at that point in time. But we neither need nor want it.

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

We could have one.

In PR#4289 Pixel Scroller I set out to introduce a GDK timer with its
own simple concept of timeslicing to gurantee screen updates every
~1/60 second. I would eventually just make sure nothing is changed
there that I don't have complete control over, rather than using
mutexes.

Then, if I in some other part of the code needed to check for
transparent possible updates in the graphics handler, I might use
repeat function calls. It would be called for, so to speak.

That's not a full argument for not using style 1, it just illustrates
the role I would assign to function calls.

> 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.

It is also a matter of straightforwardness. I would require a reason
for recalculating something at a point where recalculation is
pointless (not the other way around).

Sometimes faster code could also be a sign that you did something
proper, even if blood-optimizing currently is politically incorrect
because of Moore's Law, or because Brandon will yell at us.

The second form also opens the possibility of an interrupt modifying
punit_focus. We are not stupid and won't actually do that. The
previous sentence does not really reverse the requirement for giving
reasons.

It's fair to mention that I also prefere Ross' line of argumentation
because the consequences of those opinons accomodate my personal
style, which is entirely functional. But that's not an argument for
or against anything.


Arnstein