[Freeciv-Dev] Re: [PATCH] Corecleanup_07Part2 has been put in incoming

[Gaute B Strokkenes <gs234@xxxxxxxxx>]

On Sun, 02 Sep 2001, gs234@xxxxxxxxx wrote:

>>> +#define MAPSTEP(x,y,x1,y1,dir)     \
>>> +  do {                                \
>>> +    (x) = (x1) + DIR_DX[(dir)];       \
>>> +    (y) = (y1) + DIR_DY[(dir)];       \
>>> + /* if ((x) == -1)        */  \
>>> + /*   (x) = map.xsize-1;       */  \
>>> + /* else if ((x) == map.xsize) */  \
>>> + /*   (x) = 0;                    */  \
>>> +    (x) = map_adjust_x((x));       \
>>> +  } while (0)                       
>> 
>> IMHO MAPSTEP should be a method which does:
>> int mapstep(int old_x,int old_y,int *new_x,int *new_y,int dir)
>> {
>>      *new_x = old_x + DIR_DX[dir];
>>      *new_y = old_y + DIR_DY[dir];
>>      return normalize_map_pos(new_x,new_y);
>> }
>> 
>> With such a method we get everything which has to do with position
>> creating and checking in one method and place.
> 
> I don't see much gain in doing so.  

Allow me to elaborate.  The above would be perfectly fine if we
inlined both mapstep() and normalize_map_pos().  As long as we don't,
the perfomance loss associated with tacking two function calls on to
half of the leaf functions in Freeciv is IMNSHO not justifiable on the
gounds that you can reduce the number of places with topological
knowledge in map.[ch] from ten to five.

-- 
Big Gaute                               http://www.srcf.ucam.org/~gs234/
It's OKAY --- I'm an INTELLECTUAL, too.