[Freeciv-Dev] Re: [PATCH] is_normal_tile function

[Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>]

On Thu, Aug 16, 2001 at 07:40:03PM -0400, Ross W. Wetmore wrote:
> It can be done twice here, and that cuts the body of this function in
> half. Although going overboard with comments on trivial or prototype 
> code can be as bad, a comment here or in the one (maybe two) other
> places where this should be defined sounds like a good idea in any
> final CVS commit/production version .

No problem if a macro RANGE_CHECK gets created and every freeciv
programmer knows what it does.

> I'd actually do a single macro IS_NORMAL(BASE,POS) and explain it, then 
> replace all the x and y specific instances with this one everywhere. It
> is sort of what is already in map.h.

What is BASE?

> But you have localized the main use to a call with both x, and y
> arguments which is really the only API you want to promote except
> in the underlying implementation. So, there shouldn't be that many
> cases to consider, and the extra step is overkill unless you already 
> had this in the header.

I'm sorry I don't understand this paragraph.

> Expanded code may look nice, but having variants on the same code 287 
> times in the code base means that cleaning it up or changing it when the 
> range is not a simple linear one will be as much of a pain as some of the
> current cleanups.
> 
> Ask yourself what an is_normal_tile would be for a doubly indexed "set".

What kind of topology is a doubly indexed "set"?

> And I am surprised that unsigned vs signed compares aren't as obvious 
> a "C" technique as they seem to be :-).

I saw this the first time here. Maybe I usually don't think in such
"weird" ways. Or there wasn't so much pressure to optimize things.

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 A life? Cool! Where can I download one?