[haiku-doc] Re: Archivable.dox patch

  • From: "Niels Reedijk" <niels.reedijk@xxxxxxxxx>
  • To: haiku-doc@xxxxxxxxxxxxx
  • Date: Mon, 28 May 2007 18:11:15 +0200

Hi David

2007/5/28, David Weizades <ddewbofh@xxxxxxxxxxx>:
last one I submitted (Thanks Thom). Any comments welcome, it was a little 
trickier to go
over this document since it deals with a fairly abstract concept so I want 
comments on clarity.

Some points:
-  To provide this interface in your classes, you should publicly inherit this
-  class. You should reimplement Archive() and Instantiate(), and provide one
-  constructor that takes one BMessage argument.
-*/
-
-/*!
+  To provide this interface in your classes you should publicly inherit this
+  class. You should use Archive() and Instantiate(), and provide one
constructor
+  that takes one BMessage argument.
+*/

Replacing 'reimplement' with 'use' is wrong. As a developer you will
need to override the Archive() and Instantiate() method in order to
actually read your own messages. The act of doing that is
reimplementing the methods.

-  \param into The message you may store your object in.
-  \param deep If \c true, all child object of this object should be stored as
-    well. Naturally, only pay attention of this parameter if you actually have
-    child objects.
+  \param into The message you store your object in.
+  \param deep If \c true, all childs of this object should be stored as well.
+    Only pay attention of this parameter if you actually have child objects.

The plural of child is 'children' (not childs). Even so, I think the
phrase 'child objects' is better, because it has more meaning in
context of C++ (in my opinion).

-  \param archive The message with the data to restore an object.
+  \param archive The message with the object to restore.

I don't think it is wise to rephrase it like this. The message does
not actually contain the object, but merely data to rebuild the
object. So the old wording was technically more correct.

-  \internal This method is used to extend the API or to provide 'hidden'
-    features. Currently nothing of interest is implemented.
+  \internal This method is used to extend the API or to provide additional
+    features. Currently nothing of interest is implemented.

The problem with this phrase is that either version may be true. Even
though many Be API classes contain the Perform() method, I don't know
any that actually implements anything. I guess the intention of it was
to be able to add calls without breaking binary compatibility. Even
so, at the moment, since we don't use it, we don't know what the
meaning is. Maybe we should rephrase it to "This method is defined in
preparation for any unforseen binary incompatible API changes.
Currently nothing of interest is implemented."

-///////////////////// Global methods
+///////////////////// Global methods

According to the Documentation guidelines, delimiters should contain
five slashes at both ends, instead of ten at the beginning (or hoever
much there are).

There are two grammar mistakes (in my opinion), but I'll leave those
for the appropriate mafia.

Good luck,

Niels

Other related posts: