[Freeciv-Dev] Re: [Patch] turn_founded

[Mike Kaufman <kaufman@xxxxxxxxxxxxxxxxxxxxxx>]

On Mon, Mar 04, 2002 at 12:29:13PM -0500, Jason Short wrote:
> Mike Kaufman wrote:
> > On Mon, Mar 04, 2002 at 05:25:14PM +0100, Raimar Falke wrote:
> 
> >>I think it is bad if a value is normally >=0 and for some cities -1.
> >>
> >>
> >>>But actually, I don't think it can give a negative value.  If a city is 
> >>>founded on the first turn and then you save/reload on the first turn, 
> >>>then did_buy would have been -1 anyway.
> >>>
> >>IMHO we should insert an assert:
> >>
> >>+      if (did_buy >= 0) {
> >>  assert(game.turn>0);
> >>
> > 
> > no. this assert crap is starting to get silly. We should write the code so
> > that game turn is initialized to a non-negative number and stays 
> > non-negative.
> 
> Well, that's what this code is intended to do, and I really don't see 
> how it could fail.
> 
> The purpose of the assert is to make sure what you're certain of really 
> is true.  If we're *absolutely* certain of it, then the assert basically 
> acts as a comment to someone reading the code:

no, a *comment* is stripped out by the preprocessor at compile time. An
assert, though a macro, does a test. which means that you're wasting cycles
doing a test that you made sure (at game initialization) would never fail.

I was just commenting to raimar about this. If you are calling a function
that does one test, but you call it 1.5 billion times during the game: if
you add an assert that does one test to that function, you are now doing 3
billion tests. It does not matter whether the function is inlined or not.
The point here is to write better code.

(this code won't be called that much, but the principle is the same)

> 
>    /* this will never make the game turn drop below 0 */
>
> (but, Raimar, shouldn't it be assert(game.turn >= 0)?  Is the first turn 
> 0 or 1?)
> 
> > speaking of asserts, has anybody attempted to run an autogame with -DNDEBUG
> > lately? I didn't think so.
> 
> I did a month or so ago.  I found a whole slew of compiler warnings (now 
> fixed, I believe), and a significant improvement in runtime [1].
> 
> [1] This is mostly from all of the assert(is_normal_map_pos(...)) calls.

interesting, I can't seem to enable NDEBUG and set timeout=-1. What kind of
a feature is that?

-mike
 
> jason
>