Follow-up Comment #1, bug #13811 (project freeciv):

Some general notes:

- The #include "shared.h" in iterator.h can be
  committed separately immediately if desired.
- The string vector header+implementation and
  the changes in common/tech.c should be separate
  patch submissions.
- Use 8 spaces instead of a tab for 4 indent levels.
- No more than 2 empty lines between code parts.

Some notes about the string vector implementation:

- I suggest a shorter "class" name, e.g. 'strvec'.
  This would also help to distinguish it from
  "vectors" created with the specvec macro.
- Most other data structures in freeciv use
  X_free() instead of X_destroy().
- Since a vector generalizes the array concept
  (i.e. it supports O(1) random access), there is
  already an "iteration interface" using element
  indicies (i.e. a for loop). Hence there is no
  need to use the generic iterator interface.
- The insert_{before,after} functions should just
  take an element index instead of iterators, if
  not removed altogether (there is already an
  insert() function).

Some notes about the other changes:

- The need for static variables and consequently the
  string vector (in this patch) would be removed if
  you just made advance_name_{by,for}_player() take
  a buffer and maximum size arguments. Updating all
  callers would a bit tedious (by=12 for=57), but it
  would be the correct approach.



Reply to this item at:


  Message sent via/by Gna!

Freeciv-dev mailing list

Reply via email to