Complete.Org: Mailing Lists: Archives: freeciv-dev: July 2003:
[Freeciv-Dev] (PR#4559) citymap iterators are misnamed
Home

[Freeciv-Dev] (PR#4559) citymap iterators are misnamed

[Top] [All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
To: undisclosed-recipients: ;
Subject: [Freeciv-Dev] (PR#4559) citymap iterators are misnamed
From: "Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>
Date: Sun, 13 Jul 2003 18:18:00 -0700
Reply-to: rt@xxxxxxxxxxxxxx

The citymap iterators are horribly misnamed:

city_map_iterate(x, y) => iterates over (city_x, city_y)
city_map_iterate_outwards(x, y) => iterates over (city_x, city_y)

  These two are the same, just a different order of coordinates
  encountered.  This is a hack that allows some iterations to work
  slightly better.  And "iterate" should come last; it should swap
  with outwards.

city_map_checked_iterate(x0, y0, cx, cy, mx, my)

  What does "checked" mean?  Basically nothing.  This is the core
  iterator.

map_city_radius_iterate(city_x, city_y, map_x, map_y)

  This is just a wrapper for city_map_checked_iterate.  "Radius" has
  no meaning here.  The variables are the same as in the other, but
  with different names.  This is by far the worst...


The basic problem is a confusion of terms.  I suggest:

  citymap: the city's map area; i.e. set of tiles usable by
           the city.  Or maybe city_map?
  city_pos: The "city position" for a tile; i.e. its coordinates
            relative to the city.
  map_pos: Global coordinates representing a tile.

So our iterators then become:

  citymap_city_iterate
  citymap_city_outwards_iterate
  citymap_map_city_iterate
  citymap_map_iterate

or something like that.  citymap_city_iterate could probably be dropped.

Failing that, I would be happy with a simple rename (not removal!) of
map_city_radius_iterate.

See also PR#1233 and PR#4558.


[Prev in Thread] Current Thread [Next in Thread]