[Freeciv-Dev] Re: cartesian_adjacent_iterate in map.h

[Jason Short <vze2zq63@xxxxxxxxxxx>]

Raimar Falke wrote:

> On Tue, Feb 05, 2002 at 10:20:53PM +0000, Vasco Alexandre Da Silva Costa wrote:
>
> > Anyone objects if i change that to:
> >
> > #define cartesian_adjacent_iterate(x, y, IAC_x, IAC_y) \
> > {                                                      \
> >  int IAC_i;                                           \
> >  int IAC_x, IAC_y;                                    \
> >  CHECK_MAP_POS(x, y);                                 \
> >  for (IAC_i = 0; IAC_i < 4; IAC_i++) {                \
> >    IAC_x = CAR_DIR_DX[IAC_i];                          \
> >    IAC_y = CAR_DIR_DY[IAC_i];                          \
> >                                                        \
> >    if (!normalize_map_pos(&IAC_x, &IAC_y))            \
> >      continue;
> >
> > #define cartesian_adjacent_iterate_end                 \
> >  }                                                    \
> > }
> No. But splint has an option where it warns about constructs like
> this:
>  for(...)
>  {
>    for(...)
>    {
>      continue or break
>    }
>  }
>
> with "are you sure that you didn't mean to continue/break the outer
> loop"? I don't think it is a good idea to change freeciv to get no
> such warning at all but in the above case in can be avoided without
> cost.
In this case, there is more danger in using an if() statement than there 
is in the continue.  If you just have
  if (normalize_map_pos(&IAC_X, &IAC_y)) {

in the macro, then a foolish/careless/clever user can insert an } else { 
into the code that uses the macro, giving (probably) bad results.
This isn't really much of a concern, but it's there nonetheless...


As to the macro, I'm all in favor of simplifying it.  I'd do it as

#define cartesian_adjacent_iterate(x, y, itr_x, itr_y)
{
  adjc_dir_iterate(x, y, itr_x, itr_y, _IAC_dir) {
    if (!DIR_IS_CARDINAL(_IAC_dir))
      continue;

but I think macro wrapping like this is frowned upon :-).

jason