Complete.Org: Mailing Lists: Archives: freeciv-dev: August 2001:
[Freeciv-Dev] Re: Pop cost patch (resending via bug system) (PR#897)
Home

[Freeciv-Dev] Re: Pop cost patch (resending via bug system) (PR#897)

[Top] [All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
To: Arien Malec <arien_malec@xxxxxxxxx>
Cc: Trent Piepho <xyzzy@xxxxxxxxxxxxx>, freeciv-dev@xxxxxxxxxxx
Subject: [Freeciv-Dev] Re: Pop cost patch (resending via bug system) (PR#897)
From: Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>
Date: Thu, 23 Aug 2001 18:11:25 +0200
Reply-to: rf13@xxxxxxxxxxxxxxxxxxxxxx

On Thu, Aug 23, 2001 at 08:19:52AM -0700, Arien Malec wrote:
> 
> --- Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx> wrote:
> > city_add_or_build_error is a static method. So there should be no
> > problems. Add a comment in header and it would be fine. If there is
> > some error we get a null exception.
> 
> I'll add the documentation.
> 
> > > diff -u -r1.6 cities.ruleset
> > > --- data/default/cities.ruleset       2001/05/24 22:12:09     1.6
> > > +++ data/default/cities.ruleset       2001/08/22 21:51:25
> > > @@ -12,7 +12,7 @@
> > >  options="1.9"
> > >
> > >  [parameters]
> > > -add_to_size_limit  = 8               ; cities >= this cannot be added to.
> > > +add_to_size_limit  = 18              ; cities >= this cannot be added to.
> > >
> > >  ;
> > >  ; City styles define the way cities are drawn
> > 
> > Whats this?
> 
> Oops: testing cruft. I'll fix.
>  
> > > +static void city_build (struct player *pplayer, struct unit *punit, char
> > *name)
> > > +{
> > > +  char *city_name;
> > > +  if(!(city_name=get_sane_name(name))) {
> > 
> > I don't like assigment in a if condition.
> 
> Neither do I, but that's what it looked like before the cut 'n' paste, and I
> was trying to adapt myself to the Freeciv style (which is really every person
> to himself, I suppose). I'll fix.
> 
> > It really looks like we need a language style person.
> 
> Just a more complete style guide. Most of these things are questions of style,
> and the style used doesn't matter so much as being consistent about it. I
> prefer explicit to implicit, so I'd rather put the assignment on its own line,
> and I'd use if (pointer == NULL) and (char == '\0') rather than if (pointer) 
> or
> if (char), but I can work either way...

Since style highly depends on personal taste I foresee a huge
discussion. It is also impossible to make everybody happy. We should
make a list of open issues and then just decide. Maybe by voting and a
final decision by the maintainers?

> But some things matter a great deal: one of the really ugly things about
> Freeciv currently is that documentation on functions (if it exists) is in the
> implementation file, and not in the header file.

Yes this forces the programmer to always look at the method.

> There are certainly other big uglies out there (undocumented magic
> numbers, etc., etc.)

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
  Microsoft does have a year 2000 problem. I'm part of it. I'm running Linux.


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