[Freeciv-Dev] Re: [PATCH] more small directional cleanups

[Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>]

On Tue, Aug 21, 2001 at 11:40:17PM -0400, Jason Dorje Short wrote:
> Trent Piepho wrote:
> > 
> > On Mon, 20 Aug 2001, Jason Dorje Short wrote:
> > > I added an assertion to make sure the loop terminates (if debugging;
> > > otherwise we assume it terminates).  Bailing out won't really do any
> > > good since it would leave the game in an unstable state.
> > 
> > Here are I think better ways to do it.  My rand_neighbor function will run 
> > in
> > a bounded time, and will aways find a neighbor if one exists.
> 
> Clever.  I like it.  Although someone pointed out that this should be
> "unnecessary", the comfort of knowing that it's safe is well worth it.
> 
> > This is I think a little nicer way to do dir_get_name().
> > 
> > const char *dir_get_name(enum direction8 dir)
> > {
> >   /* a switch statement is used so the ordering can be changed easily */
> >   switch(dir) {
> >     case DIR8_NORTH:     return "N";
> >     case DIR8_NORTHEAST: return "NE";
> >     case DIR8_EAST:      return "E";
> >     case DIR8_SOUTHEAST: return "SE";
> >     case DIR8_SOUTH:     return "S";
> >     case DIR8_SOUTHWEST: return "SW";
> >     case DIR8_WEST:      return "W";
> >     default:
> >       assert(0);
> >       return "Bad Direction";
> >   }
> > }
> 
> Better because it's in the "default" case or better because it returns
> "Bad Direction"?  The second is good, the first one I find less legible.

You have programmed the same as a default case but didn't put it into
the switch statement where a programmer will expect it.

> Patch attached.

Patch applied.

> +  int dirs[8] = {0, 1, 2, 3, 4, 5, 6, 7}; /* list of all 8 directions */

You introduce new magic numbers?

        Raimar
-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 "I do feel kind of sorry for Microsoft. Their attornies and marketing
  force must have tons of ulcers trying to figure out how to beat (not
  just co-exist with) a product that has no clearly defined (read
  suable) human owner, and that changes on an hourly basis like the
  sea changes the layout of the sand on a beach. Severely tough to
  fight something like that."
    -- David D.W. Downey at linux-kernel