[Freeciv-Dev] Re: Unsafe assertions. (PR#864)

[Jason Dorje Short <jshort@xxxxxxxxxxxxx>]

Trent Piepho wrote:
> 
> On Fri, 27 Jul 2001, Thue Janus Kristensen wrote:
> > On Fri, Jul 27, 2001 at 11:24:58AM -0700, Trent Piepho wrote:
> > > On Fri, 27 Jul 2001, Thue Janus Kristensen wrote:
> > > I think Jason's way was a lot better.  You have weakened all the 
> > > assertions by
> > > only checking is_real instead of checking for is_normal.  You also 
> > > normalize
> > > every time, when the author might very well have intended to assert that
> > > normalization was NOT necessary.
> >
> > Note that I did not weaken the assertions. normalize_map_pos() returns TRUE
> > even if the tile is not normalized.
> 
> But when the author wrote the assertion, did they mean to assert that it was a
> real tile, or that it was a normalized position?  If the former, then why
> didn't they use is_real_tile to being with?  Probably because they thought
> that normalize_map_pos() would return false if the coordinates were not
> normalized.

I agree - we shouldn't try to just make the new code do the same thing
as the old, since the original writer(s) obviously did not achieve what
it was they were trying to do.

A reasonable compromise would be to assert that the tile is real and
then normalize it (as Thue's patch does) for the release, then
afterwards make things cleaner.  I believe over half of the cases do not
need to be normalized, but should just have an assertion check that they
are normal already.

jason