[Freeciv-Dev] Re: review of corecleanup_06

[Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>]

On Thu, Aug 23, 2001 at 08:19:53PM -0400, Ross W. Wetmore wrote:
> >3.  In client/goto.c what is the point of this ugly code change?
> >
> >-  for (i = goto_array_index-1; i >= first_index; i--) {
> >+  for (i = goto_array_index; --i >= first_index; ) {
> 
> Actually it is beautiful :-). Loop branching and blocks are usually
> better handled when you don't split the decrement and test steps. It
> is an automatic correction for such poor code style as the original.

I agree with Jason: this change is ugly.

> >4.  In server/gotohand.c/1190 you add a normalize_map_pos call without
> >checking it's output.  Every time a normalization is done the realness
> >should be verified also - either by checking the return value of
> >normalize_map_pos() or with an assert(is_real_tile(...)).
> 
> No! But what you are saying is the safe bet if you aren't sure.
> 
> In this case, you just need to make sure the coordinates are normalized,
> not that they are real - that is guaranted. The original comment is a clue.
> 
> But I can accept an assert as a paranoid measure at this point. It is
> just that sometime they will all need to be taken out, as the clutter of
> a lot of unnecssary checks will become excessive the way things are going.

We hope to get this cleaned up in the term so don't worry it will go
away.

> >8.  In server/cityturn.c this code also seems unhelpful.
> >
> >-        if (cur > best) {
> >-          bx=x;
> >-          by=y;
> >-          best = cur;
> >-    }
> >+        if (cur > best) 
> >+          bx= x, by= y, best= cur;
> 
> Comma operators group things that need to be grouped. It is a foolish
> style that keeps splitting things to separate lines where a misplaced
> or missing if bracket will seriously damage the required behaviour.
> 
> If you are going to program in "C" learn to use "C" is a good motto.

I agree with Jason: your changes make the code uglier.

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 "I was dead ... but I'm better now."
    -- Capitain Sheridan in Babylon 5