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

[Jason Dorje Short <jshort@xxxxxxxxxxxxx>]

Asher Densmore-Lynn wrote:
> 
> At 09:58 PM 8/15/01 +0100, you wrote:
> 
> >> > > +int is_normal_tile(int x, int y)
> >> > > +{
> >> > > +  return 0 <= y && y < map.ysize && 0 <= x && x < map.xsize;
> >> > > +}
> 
> >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 surely hope this doesn't become the standard.  Cleverness like this is
unnecessary, just like using 128>>dir instead of 1<<DIR_REVERSE(dir). 
Maintainability is far more important.  Who's to say a clever compiler
won't be able to optimize either of these expressions itself [1]?

> It took me a few seconds to get that function...
> 
> But really, all it needs is parentheses, right?
> 
> return (0 <= y) && (y < map.ysize) && (0 <= x) && (x < map.xsize);
> 
> That looks good to me... don't think we -really- need to cast everything as
> well.

The cast is an optimization:

  return (unsigned)y < map.ysize && (unsigned)x < map.xsize;

Ugly and unreadable, but possibly faster.  The speed benefit is
completely unnecessary in this case, though.

> Though for preference, I'd rearrange it.
> 
> return (y > 0) && (y < map.ysize) && (x > 0) && (x < map.xsize);

The parenthesis aren't necessary either; some people find it easier to
read with them and some find it harder.

I think the clearest way of writing it would be

  return (0 <= y && y < map.ysize) && (0 <= x && x < map.xsize);

but that's just me.

[1] Although this may sound farfetched, I don't see that it's
unreasonable.  M << (N-x) could be optimized to (M<<N)>>x so long as
((M<<N)>>N)==M.  (0 <= x && x < N) could even more easily be optimized
as (unsigned)x < N.  Why should the coder worry about this?

jason