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

[Gaute B Strokkenes <gs234@xxxxxxxxx>]

On Wed, 15 Aug 2001, hawk@xxxxxxxxxxxxxxxxxxxxxxx wrote:
> On Wed, Aug 15, 2001 at 09:58:37PM +0100, Gregory Berkolaiko wrote:
>>  --- Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx> wrote: > On Wed,
>>      Aug 15,
>> 2001 at 09:41:17PM +0100, Gregory Berkolaiko wrote:
>> > >  --- Jason Dorje Short <jshort@xxxxxxxxxxxxx> wrote: 
>> > > > +***************************************************************/
>> > > > +int is_normal_tile(int x, int y) { return 0 <= y && y <
>> > > > +map.ysize && 0 <= x && x < map.xsize;
>> > > > +}
>> > > 
>> > > you might want to use Ross' geeky way of writing such things:
>> > > 
>> > > return ((unsigned) y < (unsigned) map.ysize) && (....)
>> > > 
>> > > or am I as usual missing something?
>> > 
>> > It is interesting to see that even a simple function can be
>> > obfuscated.
>> 
>> I don't like it either but since it's virtually everywhere in Ross'
>> patch and I'm sure it's going to be accepted sooner or later, maybe
>> things should be more uniform?
> 
> I haven't looked at Ross' patch but I hope the patch didn't get
> accepted containing constructs like the above.

I have a number of issues with Ross' patch, but this isn't one of
them.  My reasoning is that although it is ugly, it is ugliness that
is easily fenced off, given a name and then explained in a comment.
Off course, it's not worth it unless you put it in a macro anyway.  In
that case, it would also help to avoid issues with side-effects of y.

Also, this patch is IMHO wrong.  A tile such as (4, -1) is, in the way
that I see things, normalised but not real.  A better name for this
function would be is_proper_tile().

-- 
Big Gaute                               http://www.srcf.ucam.org/~gs234/
Sign my PETITION.