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

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

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.

(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.)

jason