[Freeciv-Dev] Re: map_same_continent().

[Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>]

On Tue, Sep 25, 2001 at 11:53:36PM +0200, Gaute B Strokkenes wrote:
> On Wed, 19 Sep 2001, hawk@xxxxxxxxxxxxxxxxxxxxxxx wrote:
> > On Wed, Sep 19, 2001 at 01:55:44AM +0200, Gaute B Strokkenes wrote:
> >> 
> >> The function map_same_continent() is used only 7 places in the
> >> code.  Its sole task is to call map_get_continent() twice and
> >> compare the results.
> >> 
> >> On the other hand, there are lots of places that do exactly the
> >> same thing manually, either directly or by caching one of the
> >> results in a variable.
> >> 
> >> As such, I do not believe that map_same_continent() serves any
> >> appreciable purpose other than save a few characters' worth of
> >> screen real estate in just a couple of functions.  Therefore, I
> >> propose to remove it entirely, so that future generations of
> >> Freeciv source readers will have one less function to digest.
> > 
> > Pro (leave it): 
> >  - it encapsulate things (decrease the entropy) it is possible that
> 
> I don't see what is encapsulated here, apart from the fact that you
> can compare the values of the continent field in a tile struct.
> 
> By the same logic, it would be good to have functions same_unit(),
> same_city(), same_int()... [We do have same_pos() already, but in this
> case we need to take normalization into account].
> 

> Speaking more generally, there are a lot of places in Freeciv where
> someone once added something they thought was a good idea, but then
> they never put in the effort to the good idea anything other than a
> curiosity, and then those who tinkered with the code afterwards never
> took notice of it either.  In such cases, we might as well do away
> with the whole thing.

Ack. There is also the inverse case that common constructs are used a
lot in the code. We should identify them and replace them. I consider
city_map_checked_iterate, MAPSTEP and unit_flag such cases.

> The following patch is a couple of days old, but it should still
> apply, mutatis mutandis.

Go ahead and apply it.

        Raimar

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