(PR#10203) Greedy CM algorithm

"Jason Short" <[email protected]> Tue, 16 Nov 2004 21:49:58 -0800
Newsgroups gmane.games.freeciv.ai
Message-ID <[email protected]>
<URL: http://rt.freeciv.org/Ticket/Display.html?id=10203 >

> [bhudson - Sat Nov 13 23:54:24 2004]:

> 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;

Hmm, well...

Using a cast is unnecessary with a void value, and may disguise an error
if you accidentally cast the wrong thing (since generally you can cast
anything).  Casts are to be avoided.  However not using a cast here is a
bit ugly, as you see.

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

Not anymore.  It is part of C99, we have a configure check for it, and
it's used elsewhere in the code.

> 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.

Interestingly in some cases our style guidelines are impossible to
follow, and you have to pick one "guideline" to break.  I believe this
is such a case.

---

Here is a new patch.  I would like more comments, but I don't think I'm
up for writing them.  If we are to use this in 2.0 this needs to be
tested out and put into the code ASAP.

jason
cm_branch_bound3.diff (text/x-patch, 119.7 KB) - not displayed