[Freeciv-Dev] Re: [Patch] Cleanup can_establish_trade_route

[Jason Short <vze2zq63@xxxxxxxxxxx>]

Jonathan Claggett wrote:
> > From: Raimar Falke [mailto:hawk@xxxxxxxxxxxxxxxxxxxxxxx]
> >
> > On Mon, Feb 11, 2002 at 04:25:27PM -0500, Jason Short wrote:
> >
> > > Raimar Falke wrote:
> > >
> > > > The original implementation used an hack (which made
> > splint unhappy)
> >
> > > > to test for existing traderoutes. The new version is cleaner.
> > > >
> > > > Please review.
> > > I agree with Gregory - this is a significant improvement.
> > >
> > > Instead of tracking used1 and used2, why not track free1 and
> > free2 (the
> >
> > > number of free trade slots)?  Then the constant "4"
> > (MAX_NUM_TRADEROUTS)
> >
> > >  will have less of an effect.
> > So this will reduce the occurrence of "4" from 3 to 1?!
> I don't think so since free1 and free2 would still need to be initialized to
> "4" at some point.
  int free1 = 0, free2 = 0;

  /* ... */

  for (i = 0; i < 4; i++) {
    /* ... */

    if (pc1->trade[i] == 0) {
      free1++;
    }
    if (pc2->trade[i] == 0) {
      free2++;
    }
  }

  return (free1 > 0 && free2 > 0);

it's not _much_ of an improvement, I'll grant you, but I certainly feel 
it's slightly better.
> More practically, another place that would use the
> MAX_NUM_TRADEROUTES would be in the city.h code below:
>
> 223   /* trade routes */
> 224   int trade[4],trade_value[4];
>
>
> And speaking of #defines, if the "4" is replaced then the "8" ought to be
> replaced with MIN_DIST_TRADEROUTE (or is this too anal? :-).
True enough.

jason