[Freeciv-Dev] Re: (PR#3450) square_iterate and unreal positions

["Raimar Falke" <rf13@xxxxxxxxxxxxxxxxx>]

On Fri, Feb 28, 2003 at 02:03:37AM -0800, Jason Short wrote:
> Ross Wetmore wrote:
> > 
> > Raimar Falke wrote:
> 
> >>>> Ok. I rephrase this to "square_iterate should be able to catch errors
> >>>> which are made by others parts of the code and not conceal the
> >>>> errors."
> > 
> > 
> > There are no errors.
> > 
> > Let me rephrase this again for you ... there are no errors.
> > 
> > Even if there were, the purpose of square_iterate and the rest of the
> > map iterators should *not* be turned into nothing more than your personal
> > Inquisition tool for catching these boogeymen you are so afraid of.
> > 
> > These iterators serve a purpose with a well defined interface and this
> > should not be broken by people that don't fully understand it for
> > purposes that have nothing to do with their original purpose.
> 
> I'm all in favor of catching boogeymen, but not at the expense of 
> breaking other code.
> 
> When we first added the checks to the iterator macros, it was pointed 
> out (and nobody disagreed with) the fact that none of them were actually 
> needed.  They were added anyway (by Raimar and myself), as (I thought) a 
> temporary measure to catch a few extra buggy cases that might not be 
> caught in other places.  As far as I know, they have not caught a single 
> one.
> 
> So I in favor of phasing them out as needed.
> 
> As a side note, I am strongly against adding new iterators with cryptic 
> names.  And for me, the "checked" modifier is as cryptic as it gets. 
> Currently we have only one of these, and it has a very different meaning 
> for "checked" than you are using.

What about moving the CHECK_MAP_POS out of the macro in the caller?

> (I can't really tell what the "checked" in city_map_checked_iterate 
> means; I think it just means it normalizes the positions before 
> returning them.  For that matter, I can't really tell what most of the 
> city.h iterators do just by looking at the names...even after reading 
> the descriptions it is difficult to match name with description.)

I agree.

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 "Life is too short for reboots."