[Freeciv-Dev] Re: City Names Version 7
[Top] [All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
On Tue, 20 Feb 2001, Erik Sigra wrote:
> Get it at <ftp://ftp.freeciv.org/pub/freeciv/incoming/citynames-7.diff.bz2>.
Much nicer!
Still a couple of things... some i said earlier:
* use freeciv coding style please: 'indent -kr -i2'
this means code like this:
void func(int test)
{
if (1) {
foo;
} else {
bar;
}
}
case in point:
Obj *append_two_lists(Obj *x1, Obj *x2) {
if (!listp(x1)) x1 = cons(x1, lispnil);
if (!listp(x2)) x2 = cons(x2, lispnil);
if (x2 == lispnil) return x1;
else if (x1 == lispnil) return x2;
else return cons(car(x1), append_two_lists(cdr(x1), x2));
}
or:
Obj *cdddr(Obj *x) {return cdr(cdr(cdr(x)));}
* i think server/util.c can be eliminated/merged with the rest of the
files, it's kind of puny.
* there's some hashtable & linked list code there that could use freeciv
functions.
* when having stuff like this:
char *c_string(Obj *x) {
if (x->type == STRING) {
return x->v.str;
} else if (x->type == SYMBOL) {
return x->v.sym.symentry->name;
} else {
/* (should be internal_type_error?) */
type_warning("c_string", x, "string/symbol", lispnil);
return "";
}
}
IMHO a switch() is nicer.
* comments before functions should be like this:
/**************************************************************************
Returns the number of bytes still unread by pack_iter.
That is, the length minus the number already read.
If the packet was already too short previously, returns -1.
**************************************************************************/
not like this:
/* Make a game module for the given name and maybe bolt it into the include
list of another module. */
* more variables & functions could be made static to avoid namespace
pollution.
* some function & var names could be better.
I'm sorry for giving you so much trouble but IMHO it's better to fix stuff
like this before rather than after commiting to CVS.
----
Vasco Costa
|
|