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

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

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

Vasco Alexandre da Silva Costa wrote:

>>section in doc/CodingStyle.  The fact that you people (tm) keep putting
>>in new functions without headers doesn't change this.
> 
> 
> Real Programmers (tm) don't write comments. That is for Quiche Eaters.
> Real Programmers (tm) don't eat Quiche.
> 
> ;-)

Mmm, quiche.

>>We need to decide whether to use "building" or "improvement".  The
>>current standard is "improvement" so it should be
>>get_improvements_with_effect() or get_imprs_with_effect().
> 
> 
> "improvement" isn't as much a standard as simply more used. Otherwise you
> wouldn't have functions like "city_got_building".
> 
> I like "building" better, but I'm not one to mince with words. Pick one
> standard, replacing is easy as this stage.
> Heck, I have already renamed everything about half a dozen times, one
> more time won't even make a difference. :-)

I also prefer building.  I had thought improvement was the universal 
standard, but you have a good point with city_got_building.  However 
there is improvement.[ch] which is a bit hard to rename.

So can we call them buildings from now on, and let improvement.c remain 
as is?

>>source_value() is not descriptive enough IMO (not that source_power() is
>>either).  I'd suggest something better but I have no idea what this
>>function does.
> 
> 
> It sums up all the contributions of a given effect type for a building and
> spits out the result. I only named it source_power() because you already
> had get_cathedral_power and get_temple_power() in common/city.c.
> 
> If this is sounding vaguely familiar, it is because I already said it.
> That function actually *is* commented, I told you comments didn't add
> anything of value 90% of the time :-).
> The bloody function body has like 9 measly lines of code.

Heh heh.  What about get_building_effect_power()?  You can rename 
"power" with "value" if you want, or drop the "get".  Or maybe even drop 
the "building".

The function body may be only 9 lines of code, but the prototype is an 
obnoxiously huge 6 lines of code...

jason