Crossfire Mailing List Archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Coding style suggestion




To the Crossfire coders:

Recently I've been delving more and more into the code and I'm 
pleased to say that in general it's in better shape than the 
typical "many-developer" public domain game one finds floating
around the net.  However, I'd like to make one suggestion for
a change in coding style that I think would go a long way towards
improving Crossfire's long term stability and the ease of adding
new features.

Currently there are many assumptions about the contents of various
fields in the object structure.  Access to values is (by and large)
done through direct use of the ->, . and = operators.  For example 
if you want to find out a players strength you write something like:

   foo = pl->stats.Str
  
I believe that it's better to use a method to access object attributes,
and not having the luxury of C++ operator overloading we're left
with using macros or functions to implement them.  I would prefer to
see something like:

   #define get_player_strength(PL) ((PL)->stats.Str)

   foo = get_player_strength(PL);

This has many benefits, I'll try to outline a few:

   - If the object structure changes (say we introduce a new level
     of structure hierarchy containing the attributes, so the
     access is now object->stats->atts.Str) you only need to change 
     one macro in one place, not numerous individual lines of code.

   - If you actually need a method you can implement one by swapping
     a function for the macro.  For example if we wanted strength to
     decrease with a players age (say 5% per year over 50) you could
     simply write a get_player_strength() function that implemented 
     this algorithm.  Otherwise you would have to add another piece
     of code after each use of the Str field.

   - A lot of object types use the fields in strange ways.  For example
     runes use stats.Cha to determine the chance of their being seen.
     This code would be much clearer if a macro called get_rune_visibility()
     was used to access this field.

   - It's easier to turn massive amounts of debugging code on and off. 
     For example you could add a check in the get_player_* macros to
     ensure that the object passed in is in fact of type PLAYER.

--Ken  

+------------------------+---------------------------------------------+
| Ken Woodruff           | Most Latin words in -us have plural in -i,  |
| woodruff@cadence.com   | but not all, & so zeal not according to     |
+------------------------+ knowledge issues in such oddities as hiati, |
| Disclaimer: What tote  | octopi, omnibi, & ignorami; ...             |
| bag full of $20 bills? |     Fowler, "Modern English Usage"          |
+------------------------+---------------------------------------------+