Complete.Org: Mailing Lists: Archives: freeciv-dev: July 2004:
[Freeciv-Dev] Re: (PR#8754) effects patch
Home

[Freeciv-Dev] Re: (PR#8754) effects patch

[Top] [All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
To: vasc@xxxxxxxxxxxxxx
Subject: [Freeciv-Dev] Re: (PR#8754) effects patch
From: "Mike Kaufman" <kaufman@xxxxxxxxxxxxxxxxxxxxxx>
Date: Mon, 19 Jul 2004 10:11:28 -0700
Reply-to: rt@xxxxxxxxxxx

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

On Mon, Jul 19, 2004 at 09:46:36AM -0700, Vasco Alexandre da Silva Costa wrote:
> 
> > On Mon, 19 Jul 2004, Vasco Alexandre da Silva Costa wrote:
> > > > all functions in effects.c should have comments in the their headers.
> > >
> > > Uh, this isn't true for the rest of the code, I don't see why the effects
> > > code should be special in this regard.
> >
> > Bad excuse. I think we agreed that this should be the case for all new
> > code, but I am not sure if this got codified in the style guide yet.
> 
> It isn't a bad excuse when pre-existing code doesn't have comments on all
> functions.
> 
> I always prefered reading code instead of comments for short functions anyway.
> And good function declarations summarize what the function does 90% of the 
> time.
> 
> I have seen over commented code. It is not easier to read. Comments work
> best when used sparingly.

nice try. this is not the case here. Please comment the code so I can
understand it with out taxing my brain.

> > BTW, a continent-wide uniqueness would be nice.

before this happens, some thought about information leakage should be done.
This has the same potential problems as CPOV vs SPOV.

> I think I will remove all over city range checks. Both these ways of
> checking an effect are equivalent:
> 
> 1) All city Temples have +2 bonus if that player has an Oracle.
> 2) An Oracle gives +2 bonus to all player cities with a Temple.

these are not necessarily equivalent if you talk about percentages and
pre- vs. post-multiplication.

-mike




[Prev in Thread] Current Thread [Next in Thread]