[Freeciv-Dev] Re: [Patch] turn_founded

[Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>]

On Mon, Mar 04, 2002 at 11:49:56AM -0600, Mike Kaufman wrote:
> 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.

Comments are dead. Assert are checked. IMHO the way should be to
include asserts by default. Later if a profiling shows that a certain
assert hurts performance you can start thinking about it. What are the
chances that it triggers? Can it be translated to a comment? Can it be
made conditional (like CHECK_MAP_POS)?

So you say that the assert in contains_special is to expensive? Have
you removed it and measured the difference?

> >    /* 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?

Since it rules out any client this feature isn't intended for
production use. The condition may be changed to IS_DEVEL||IS_BETA but
the idea is correct.

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 "The BeOS takes the best features from the major operating systems. 
  It's got the power and flexibility of Unix, the interface and ease 
  of use of the MacOS, and Minesweeper from Windows."