[Freeciv-Dev] Re: [PATCH] Small improvement at genlist and maphand

[Trent Piepho <xyzzy@xxxxxxxxxxxxx>]

On 20 Aug 2001, Markus Linnala wrote:
> Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx> writes:
> 
> > It is. I would like a discussion what is preferred: id or pointer. If
> > one type is selected we can start our crusade.

Pointer is better.  The id is just there for external data representation and
because giving players some kind of ordinal number is useful for some things. 
Just like units and cities, pass a player to function with a pointer.  It looks
like this is the way used in almost all of the code.

> struct player *get_player(int player_id)
> {
>     assert(player_id >= 0 && player_id < game.max_players);
>     return &game.players[player_id];
> }

I suppose you can put the assert in there, but as far as I know, bad player
IDs isn't a problem.  Positions are often wrong, since they are often
generated by code that doesn't check properly.  player_id's aren't generated
so there really is no opportunity to get them wrong.


> int get_player_by_id(struct player *pplayer)
> {
>     assert(pplayer->player_no >= 0 && pplayer->player_no < game.max_players);
>     return pplayer->player_no;
> }

This function really makes no sense.  What is that assert checking?  That
the player structure pointed to really points to a player structure?  It's
acceptable to de-reference a player pointer to get at the fields, no need
to create accessor functions for everything.