[OpenBabel-Devel] API-review; ideas for better interface-documentation. Was: forcefield classes

[Carsten N. <cni...@gm...>]

Am Sonntag, 18. Februar 2007 05:00 schrieb Tim Vandermeersch:
> The ghemical  forcefield is now completely implemented. 

> All the energy expressions and graphs on the wiki for the ghemical
> forceifeld are also finished.

Tim, I read a bit of your code today. A lot API-docs are missing. Look for 
example here and you'll notice a lot gaps:

http://openbabel.sourceforge.net/dev-api/classOpenBabel_1_1OBForceField.shtml

I hope it is ok that I comment a bit about this here: For example, in 

  svn/openbabel/src/forcefields/forcefieldghemical.h

you have a lot code similar to this:

  class OBFFVDWCalculationGhemical : public OBFFCalculation
  {
    public:
      OBAtom *a, *b; // atoms of the pair
      double ka, Ra, kb, Rb, kab, rab, sigma, sigma6, sigma12;

      double GetEnergy();
      vector3 GetGradient(OBAtom *atom);
  };

Four comments:
1. If doxygen works the way I think it works this class (actually, the 6 
classes derived from OBFFCalculation in that file) will not be picked up by 
doxygen because the class itself is not wrapped in doxygen.

2. I doubt that the OBAtom's and the double's need to be public. private 
should be ok, right?

3. Both methods (Get*) are not documented. As stated in the first comment I 
think they will not be correctly displayed in the APIdocs... please add 
a "@return ..." comment, if possible (or tell me what they do and I will send 
you a patch)

4. I am not sure if the name OBFFVDWCalculationGhemical is a good one. After 
reading the code I _think_ you moved the code from Ghemical to OB or 
something like that but the code is not really related to Ghemical. In other 
words: The name shouldn't contain the string "Ghemical" because there is no 
relation.

5. I found this page in the wike:
  
  http://openbabel.sourceforge.net/wiki/Molecular_mechanics

I really think that these information belong into the code (of course as 
doxygen-comments!) so that I would have found them ealier and they are easier 
to be keept uptodate. Doxygen can be a really powerful tool. I think that 
specific page should be move into a "Mainpage.dox"-file which would result in 
a page similar to 

  http://api.kde.org/cvs-api/kdelibs-apidocs/phonon/html/index.html
or
  http://api.kde.org/cvs-api/kdelibs-apidocs/khtml/html/index.html

Look especially at the Phonon-page. It is the perfect example of really really 
good API-documentation which looks good and is interlinked a lot. 
You can see how such a Mainpage.dox is done for example here:

http://websvn.kde.org/trunk/KDE/kdelibs/phonon/Mainpage.dox?rev=630516&view=markup

I am of course willing to help you! I think that API-docs are _really_ 
important. I also think this because I intend to use OB more and more in the 
future and that I am currently a bit lost in the API ;-)

Carsten