[Freeciv-Dev] Re: Inconsistent function names and arguments

[Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxxxx>]

On Mon, Jul 24, 2000 at 06:56:44AM -0400, Jeff Mallatt wrote:
> At 2000/07/22 14:42 , Raimar Falke wrote:
> >Examples 1: There are two function in map.h
> >map.h:int is_water_adjacent_to_tile(int x, int y);
> >map.h:int is_water_adjacent(int x, int y);
> 
> Well, is_water_adjacent(x,y) just seems totally wrong, because it includes
> the tile at (x,y).  If it did not include the tile at (x,y), it should be
> named similarly to the is_*_near_tile() functions (i.e.,
> is_water_near_tile()).
> 
> And, is_water_adjacent_to_tile(x,y) is really more like
> is_irrigation_source_available(x,y).

Exactly. To we can define functions like:
  int is_water_at_tile(int x, int y)
  int is_water_adjacent(int x, int y)
  int is_irrigation_source_at_tile(int x, int y)
  int is_irrigation_source_adjacent(int x, int y)
the *_adjacent functions can be reduced to the *_at_tile functions.

> >Example 2: There is a function
> >government.h:int government_has_flag(const struct government *gov, 
> >                                     enum government_flag_id flag);
> >and there are functions
> >tech.h:int tech_flag(int tech, int flag);
> >unit.h:int unit_flag(Unit_Type_id id, int flag);
> >
> >Why not be consistent and call the last two tech_has_flag and unit_has_flag.
> >The latter name should be changed another time to unit_type_has_flag.
> >Nevertheless would it be useful to define a unit_has_flag since a lot of the
> >usages of unit_flag today are "if (unit_flag(punit->type, F_SOME_FLAG))".
> >Last point: the government function takes a pointer while the
> >others takes ids. (And there is the another issue int-vs.-real type for the
> >flag)
> 
> How about replacing the tech and unit functions with:
> 
> tech.h
> ------
> 
> /* same as current tech_flag() */
> int tech_type_has_flag(Tech_Type_id tech_type, enum tech_flag_id flag);
> 
> unit.h
> ------
> 
> /* same as current unit_flag() */
> int unit_type_has_flag(Unit_Type_id unit_type, enum unit_flag_id flag);
> 
> /* this takes a pointer to a unit, not to a unit type */
> int unit_has_flag(struct unit *punit, enum unit_flag_id flag);
> 
> Many calls currently look like:
>    unit_flag(punit->type, ...)
> replace them with:
>    unit_has_flag(punit, ...)
> 
> And, replace all the rest of the unit_flag() calls with unit_type_has_flag().

This were the changes I had in mind. I thing is IMHO missing: the difference
between poiners and ids:

int unit_has_flag(Unit_Type_id unit_id, enum unit_flag_id flag);
int punit_has_flag(struct unit *punit, enum unit_flag_id flag);

So calls like:
  unit_flag(punit->type, ...)
can be replaced with:
  unit_has_flag(punit->id,...)
or
  punit_has_flag(punit,...)

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 "We just typed make..."
    -- Stephen Lambrigh, Director of Server Product Marketing at Informix,
                         about porting their Database to Linux