[Freeciv-Dev] Re: Style question

[Petrus Viljoen <viljoenp@xxxxxxxxxxx>]

Raimar Falke wrote:

>
> +  /* we haven't found a name: it's a normal tile or they're all used */
> +  for(nptr=nation->default_city_names; *nptr; nptr++) {
> +    if(!game_find_city_by_name(*nptr))
> +      return *nptr;
> +  }
>
> I think that this is unnecessary code duplication. So was thinking about a
>
> #define SEARCH_AND_RETURN_CITY_NAME(list)   \
>     for(nptr=list; *nptr; nptr++) {         \
>       if(!game_find_city_by_name(*nptr)) {  \
>         return *nptr;                       \
>       }                                     \
>     }
>
> Which would yield:
>
>   /* deal with rivers */
>   if(map_get_special(x, y) & S_RIVER) {
>     if(is_terrain_near_tile(x, y, T_OCEAN)) {
>       /* coastal river */
>       SEARCH_AND_RETURN_CITY_NAME(nation->default_crcity_names)
>     } else {
>       /* non-coastal river */
>       SEARCH_AND_RETURN_CITY_NAME(nation->default_rcity_names)
>    }
>  }

.............

>
> Is this a good idea? Comments?
>

Using a function should result in a smaller working set. And the code can be 
reused
without dependencies on variable names.

<DataPtr>  SearchAndReturnCityName( <DataPtr> *list) {
    while (!(*list) && (!game_find_city_by_name(*list)))  {
        list++;
    }
    return *list;
}


OR

<DataPtr>  SearchAndReturnCityName( <DataPtr> *list) {
    while (!(*list) && (!game_find_city_by_name(*(list++))))  ;
    return *list;
}

AND the  macro ( Should be undef 'ed   after the function using it, since it 
depends
on a variable with the name nptr !!)

#define SEARCH_AND_RETURN_CITY_NAME(list)   \
    if ((nptr = SearchAndReturnCityName(list)))  { \
       return *nptr; \
    }

Well , I'm not so sure about the smaller working set , but it is more reusable.