Re: (PR#10203) Greedy CM algorithm

<URL: http://rt.freeciv.org/Ticket/Display.html?id=10203 >

On Fri, Nov 12, 2004 at 10:18:17PM -0800, Jason Short wrote:
> 
> <URL: http://rt.freeciv.org/Ticket/Display.html?id=10203 >
> 
> > [bhudson - Thu Oct 14 18:52:59 2004]:
> > 
> > > > Also, can you send a patch that does not include the old cm code,
> > and
> > > > that makes the necessary changes to the rest of the code
> > > > (auto_arrange_workers)?
> > 
> > Here's the most minimal patch: it changes only cm.c and leaves the API
> > untouched.
> > I went through and cleaned up the comments and a couple violations of
> > the coding standards.
> 
> I read over the new code.  In the process I edited it: fixing style,
> adding more comments, replacing loops with iterators, etc.  Pretty much
> all changes are cosmetic.

I don't understand why you have
+  struct cm_tile_type * const *a = va;
rather than
-  const struct cm_tile_type *a = *(struct cm_tile_type **)va;

I thought this was a gcc extension:
+  bool marked[lattice->size];

A couple of the comments are wrong:
- take_sibling_choice doesn't take a comparably-good choice, but rather
  replaces the current top of the choice stack with another choice
  expected to be worse.  Really this function should be called
  take_sibling_branch.
- get_city_surplus: the thing about "pointer values" is incorrect: it's
  returning surplus, happy, and disorder by reference.

The "fixed" indentation of arguments is wrong in a couple places, like
in compute_fitness.

Otherwise, it looks better after having been cleaned up.


> I can't say I entirely understand it.  But then the current code is even
> worse.  It does seem pretty clear that the B&B algorithm will be a lot
> less bug-prone than the current method.  So unless anyone objects I will
> commit this patch (Benoit, make more changes to it if you want).

I don't have time to work on this at the moment (or for the forseeable
future) but I'll happily answer questions.

        -- Benoît