[Freeciv-Dev] Re: (PR#13460) consistent names for index-to-pointer lookups

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

<URL: http://bugs.freeciv.org/Ticket/Display.html?id=13460 >

Benoit Hudson wrote:
> <URL: http://bugs.freeciv.org/Ticket/Display.html?id=13460 >
> 
>>Opinions?
> 
> 
> get_player versus find_city_by_id is my pet peeve.  Whatever the
> naming convention is, it should be consistant for all int -> something
> mappings.  I think get_X_by_id or find_X_by_id makes me happiest. 
> Especially when we're dealing with integer types where type-checking
> is pretty weak, I like the idea of having the name of the function
> tell me what type to put in.

Well there is a distinction between players/tiles/etc and
units/cities/connections.  The latter are both non-contiguous and
non-static.  The non-contiguous part is why they have an id rather than
an index and why you have to do a find() rather than a get() - in fact
for connections the find() is actually O(n) since there's no hash of
them.  And yes, this is really a backend issue that the user shouldn't
have to worry about.  However the non-static part (which is closely
related to the non-contiguous property of course) means you can't keep
around pointers to these objects and expect them to still be there.  In
the absence of garbage collection you have to store the ID and redo a
find_xxx_by_id each time.

Honestly I don't mind having get_XXX_by_index for some objects and
find_xxx_by_id for others; if nothing else this helps you remember which
can be stored by pointer and which must use the ID.  What I do mind is
the 5 or so different forms that we have now for the contiguous objects.

> I don't understand how get_unit_class fundamentally differs from the others.

It should properly be called get_class_for_unit_type.  It takes a unit
type not a class index.  And yes I also didn't understand this at first
so it is very dangerous since it's easy to misuse.  Currently there is
no index lookup function for the unit class.

-jason