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

[David Pfitzner <dwp@xxxxxxxxxxxxxx>]

Jeff Mallatt wrote:
> At 2000/07/24 09:37 , Raimar Falke wrote:
> >On Mon, Jul 24, 2000 at 06:56:44AM -0400, Jeff Mallatt wrote:

> >> /* same as current tech_flag() */
> >> int tech_type_has_flag(Tech_Type_id tech_type, enum tech_flag_id flag);

> >> /* 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);

> >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);
> 
> This difference is the difference between ????_has_flag and
> ????_type_has_flag in my proposal.  It's just subjective, but I don't like
> the name punit_has_flag.

I agree about the name "punit_has_flag", in that currently function 
names don't use the form "punit" (or p-anything), only variables use 
that.

But I'm guessing maybe there is confusion above between unit types 
(type Unit_Type_id, valid range 0 to <U_LAST) and unit ids (currently 
type int, range 1 to about INT_MAX (?)), and Raimar's example 
should perhaps more clearly be the 3 functions:

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

?

But if so I don't much like the idea of having a unit_id version -- 
better for the caller to call find_unit_by_id() and then use the result 
(which encourages the caller to do this once, and cache the result). 
And if we were to have a unit_id version, for better consistency with 
the bulk of existing code, I would prefer the names be, eg:

  int unit_type_has_flag(Unit_Type_id unit_type, enum unit_flag_id flag);
  int unit_has_flag(struct unit *punit, enum unit_flag_id flag);
  int id_unit_has_flag(int unit_id, enum unit_flag_id flag);

-- David