need feed back about the comments I write

[lawrence]

I've been bad about documentation so far but I'm going to try to be
better. I've mostly worked alone so I'm the only one, so far, who's
suffered from my bad habits. But I'd like other programmers to have an
easier time understanding what I do. Therefore this weekend I'm going
to spend 3 days just writing comments. Before I do it, I thought I'd
ask other programmers what information they find useful.

Below is a typical class I've written. Besides simply writing comments
on all the methods, and besides keeping things up to date (I already
know I should do these things) is there any other style tips or advice
you'd like to offer? No ad homenien attacks, no flame wars please,
just constructive criticism.





<?php

class McSelect {


// 03-06-04 - the 2 helper objects
var $controllerForA ll = null;
var $resultsObject = null;


// 03-06-04 - the 2 worker objects
var $queryObject = null;
var $datastoreResul tsObject = null;


// 11-04-03- I don't have time to implement this today, but this
object should take a secret password from the config
// file and hand it to the queryObject. The queryObject should check
it against the config file. This would be a
// security measure, to make sure the queryObject is never called
from outside of this object.
var $secretQueryObj ectCheckCode;







/**
* 11-04-03 - constructor
*
* McSelect contains two main member objects, a query object that
makes calls to a datastore and returns a pointer,
* and a datastore results object that uses that pointer to get
information. McSelect always loads a datastore specific
* subclass of the generic parent classes McQueryObject and
McDatastoreResu lts.
*
* Our goal is to mask the final code from what datastore is being
used. We should be able to switch from
* a MySql database to a PostGreSql database or an XML file without
having to change any of the end calls. So all
* calls should go through this object, which looks in the config file
to find out what the default datastore is for
* this website.
*
* $datastoreResul tsObject - this is the object that holds the methods
that uses the resource pointer to get actual data out
* of the retrieved dataset. Each type of datastore (PostGreSql,
MySql, XML flat file, etc) should have its own $getterDataObje ct
*
* This class last edited: 03-17-04
*
* Normally, when using the select object in code, its use will look
something like the following (this from the function
showRecentWeblo gEntries2):
*
* global $controllerForA ll;
* $selectObject = & $controllerForA ll->getObject("McS elect", " in
showRecentWeblo gEntries2().");
*
* $selectObject->setDatastoreOb ject();
* $selectObject->setQueryObject ("GetAllOfTypeR eversed");
* $selectObject->setInfoToBeSou ght("weblogEntr ies");
* $selectObject->getInfo();
*
* $howMany = $selectObject->getCountOfRetu rn();
*
* The first method, setDatastoreObj ect, when not given a parameter,
sets the datastore to the default, as defined in
* the config file. Right now, that would often be MySql. The next
method, setQueryObject, gets a subclass of McQuery and
* loads that as a member object of McSelect. If you're making a call
to a database, this would usually be SQL, if you're
* making a call to a flat file, this would be a file address. The
next method, setInfoToBeSoug ht, allows you to pass up
* to 3 parameters to your query. If you need more than 3 parameters,
I suggest you rewrite you query. Lots of specific
* queries are good as they help to insulate the code against future
changes in the datastore design. For instance, I question
* whether meta information like keywords should be stored in
mcContentBlocks , since it isn't content. So possibly in the
* future keywords will be in another table. If calls to select
keywords have their own query (their own query object) then
* making that change in the future will involve changes only in those
queries. In other words, I recommend against
* using GetAllOfType, setInfoToBeSoug ht("keywords") , and instead
favor GetAllKeywords.
*
* The function getCountOfRetur n() allows you to use the selectObject
from inside of a for loop, like here:
*
* for ($i=0; $i < $howMany; $i++) {
* $row = $selectObject->getRowAsArrayW ithStringIndex( );
* if (is_array($row) ) extract($row);
* print $cbHeadline;
* }
*
* public
* returns void
*/
function McSelect() {
$this->controllerForA ll = & $GLOBALS["controllerForA ll"];
$this->resultsObjec t = &
$this->controllerForA ll->getObject("McR esults", " in the constructor
for McSelect.");
}








/**
* 03-17-04 - setter
*
* This method is wholly for debugging purposes. A common source of
bugs is the wrong type of parameter being passed into
* a query object, or the parameters being passed in in the wrong
order. The query objects are good about outputting error
* messages explaining the problem, but it is often hard to find out
where the select object is being called from. The
* query object might tell you that the first parameter is a string
but needs to be numeric, but where is the problem happening?
* getRecentEntrie s()? printSelectToSc reen() in some McForm subclass?
Where? This method exists to keep a record of where
* things are being called from. Just type the name of the function or
method and class. When debugging you can go into
* the config file and set these error messages to print to the
screen.
*
* @param - $message - string - NOT OPTIONAL - this should be the name
of the function that is using the select object, like
* for instance "getRecentEntri es". If a class method,
"printSelectToS creen in McFormsNavigati on".
*
* public
* returns void
*/
function begin($message= false) {
if ($message) {
$this->resultsObjec t->selectNotes($m essage);
} else {
$this->resultsObjec t->addToErrorMess ages("In begin(), in McSelect,
we expected a message to be given to us, but there was none.");
}
}






/**
* 03-06-04 - setter of McQueryObject subclass
*
* $queryObjectNam eSuffix and $queryObject are two strings which get
combined to specify which subclass of McQuery should get loaded.
* $queryObjectNam eSuffix contains the datastore in use (like "MySql"
or "Xml"), where as $queryObject merely contains the query
("GetAllOfType" ).
* $controllerForA ll is then used to create an instance of the query
object, which becomes a class instance belonging to McSelect.
*
* @param - $queryObject - string - sets the second half of the query
object name
*
* @param - $queryObjectNam eSuffix - string - This parameter can be
used to override what is set in the constructor. Sets the first half
of the query object name.
*
* public
* returns void
*/
function setQueryObject( $queryObject=fa lse,
$queryObjectNam eSuffix=false) {
if (is_string($que ryObject)) {
if (!$queryObjectN ameSuffix) {
// 03-06-04 - normally $queryObjectNam eSuffix is false so we
construct the string ourselves. We have adjusted this
// method so that programmers can override the built-in choice,
which is important if they wish to switch from one datastore
// to another in the middle of the code.
// 11-04-03 - we want to enforce a naming convention as regards
query objects
$config=getConf ig();
$queryObjectNam eSuffix = $config["defaultQueryOb jectPrefix"];
}
$query = "McQueryObject" .$queryObjectNa meSuffix.$query Object;
$this->queryObject = & $this->controllerForA ll->getObject($que ry, "
setQueryObject( ), inside of McSelect");

if (is_object($thi s->queryObject) ) {
$this->queryObject->setDatastoreCo nnector($queryO bjectNameSuffix );

$className = get_class($this->queryObject) ;
$this->resultsObjec t->selectNotes("I n setQueryObject( ), in the
class McSelect, we just set a query with the name '$className.' Also,
we just attempted to set the datastore connector using the '
$queryObjectNam eSuffix ' prefix.", "McSelect") ;
} else {
$this->resultsObjec t->error("In setQueryObject( ), in the class
McSelect, we tried to set a query object called '$query', but for some
reason we couldn't.", "McSelect") ;
}
} else {
$this->resultsObjec t->error("In setQueryObject, in the class
McSelect, we expected the first parameter to be a string, but it was
not. It needs to tell us what query object to look for.", "McSelect") ;
}
}



function setQuery($query Object) {
// 11-04-03 - alias of above method
$this->setQueryObject ($queryObject);
}










/**
* 11-04-03 - obviously, this is just a wrapper for a method in the
queryObject
*/
function setInfoToBeSoug ht($infoToBeSou ght1=false,
$infoToBeSought 2=false, $infoToBeSought 3=false) {
$this->queryObject->setInfoToBeSou ght($infoToBeSo ught1,
$infoToBeSought 2, $infoToBeSought 3);

$className = get_class($this->queryObject) ;
$this->resultsObjec t->selectNotes("I n setInfoToBeSoug ht in the class
McSelect this is our first value: $infoToBeSought 1 and this is out
second: $infoToBeSought 2 and this is the query object name:
$className");
}






/**
* 03-17-04 - getter and setter
*
* This method runs a query using the query object and gets back a
pointer to a datastore return. It then gives the
* pointer to the datastore results object. This method does not
return any info, for that, use getRowAsArrayWi thStringIndex() ,
* getResultAsArra yWithStringInde x(), or getJustOneField ().
*
* public
* returns void
*/
function getInfo() {
// 03-17-04 - for debugging, set printQuery to true in the config
file and this will print the query on screen.
$this->queryObject->printQuery() ;

// 03-17-04 - this next line is the one that actually makes the call
to the datastore.
$dsResultPointe r = $this->queryObject->getInfo();

if (is_resource($d sResultPointer) ) {
$className = get_class($this->queryObject) ;
$this->resultsObjec t->selectNotes("I n the method getInfo(), in the
class McSelect, the query object called $className just returned a
pointer to a database resource.");

// 03-06-04 - I hate this next line, which I'm adding today, but
I've had trouble getting data back the getInfoObject, because I've
been passing pointers
// as references and for some reason it isn't working. So I'm
changing everything today. From now on the final object will get the
pointer right here, and
// all manipulations will be done by the final object.
$this->datastoreResul tsObject->setInfoResourc ePointer($dsRes ultPointer);
} else {
$className = get_class($this->queryObject) ;
$query = $this->queryObject->getQuery();
$this->resultsObjec t->error("In the method getInfo(), in the class
McSelect, we assume we are getting back a database resource pointer
from the query object called '$className'. However, we are not getting
anything back. This is the query: $query", "McSelect") ;
}
}








/**
* 03-12-04 - setter
*
* This method should always be the first one called when using the
select object. Used correctly, it tells the select
* object whether to get information from a database, flat file, or
XML stream.
*
* The software needs to draw information from diverse sources -
databases, flat files, XML streams, etc. The Select
* object gets them all, by using different query and datastore
results objects. With a database, the query object runs a
* call against a database and returns a pointer to database resource.
With files, the query object opens a file and
* returns a pointer to that file. (If a file) the
McDatastoreResu ltsFile would then be able to get data out of that
pointer.
*
* @param - $objectSuffix - optional - if present, this is what we
attach to the string "McDatastoreRes ults" and then
* try to find a class by that name. A common example might be "File"
which then becomes "McDatastoreRes ultsFile".
* This would be for getting data from a file pointer. "MySql" would
lead to "McDatastoreRes ultsMySql" and could get
* data back from database return.
*
* public
* returns void
*/
function setDatastoreObj ect($objectSuff ix=false) {
// 03-16-04 - these next 2 lines are merely to make sure the old
objects are destroyed everytime this method is called.
if ($this->queryObject) unset($this->queryObject) ;
if ($this->datastoreResul tsObject)
unset($this->datastoreResul tsObject);

if (is_string($obj ectSuffix)) {
$name = "McDatastoreRes ults".$objectSu ffix;
$this->datastoreResul tsObject = &
$this->controllerForA ll->getObject($nam e, " in setDatastoreObj ect() of
McSelect.");
} else {
$config = getConfig();
$objectSuffix = $config["defaultQueryOb jectPrefix"];
$name = "McDatastoreRes ults".$objectSu ffix;
$this->datastoreResul tsObject = &
$this->controllerForA ll->getObject($nam e, " in setDatastoreObj ect() of
McSelect.");
}
}








/**
* 11-08-03 - We cannot call getRowAsArrayWi thStringIndex() unless we
know how many items there are in the return.
* Actually, there are other ways, but this is one.
*/
function getCountOfRetur n() {
$count = $this->datastoreResul tsObject->getCountOfRetu rn();
return $count;
}







/**
* 03-16-04 - wrapper of getter
*
* We tell the datastore object to take the resource that points to
the returned dataset and get back a row.
* We are using the resource pointer to the returned dataset to fetch
the next row and return it with a string index
*
* public
* returns array
*/
function getRowAsArrayWi thStringIndex() {
if (is_object($thi s->datastoreResul tsObject)) {
$row = $this->datastoreResul tsObject->getRowAsArrayW ithStringIndex( );

$className = get_class($this->datastoreResul tsObject);
if (is_array($row) ) {
$this->resultsObjec t->selectNotes("I n
getRowAsArrayWi thStringIndex() , in the class McSelect, we expected the
data object $className to return a row of data to us (as an array),
and it has.");
return $row;
}
} else {
$this->resultsObjec t->error("In getRowAsArrayWi thStringIndex() , in
the class McSelect, we'd like to get some data from the datastore
results object, but there is no such object. Was setDatastoreObj ect()
called?", "McSelect") ;
}
}






/**
* 11-04-03 - wrapper of getter
*
* We take the resource that points to the returned dataset and feed
it into a method of $formatDatastor eResults.
* We are taking the whole of the returned dataset and putting in an
array with a string index and returning that array
*
* public
* returns array
*/
function getResultAsArra yWithStringInde x() {
if (is_object($thi s->datastoreResul tsObject)) {
$dsArrayWithStr ingIndex =
$this->datastoreResul tsObject->getResultAsArr ayWithStringInd ex();
return $dsArrayWithStr ingIndex;
} else {
$this->resultsObjec t->error("In getResultAsArra yWithStringInde x(),
in the class McSelect, we wanted to get some data from the datastore
results object, but it didn't exist. Was setDatastoreObj ect()
called?", "McSelect") ;
}
}






/**
* 12-24-03 - wrapper of getter
*
* Some calls to the datastore only get back one field. This method is
to offer the convenience of returning
* just the content of the field, rather than the one-dimensional
array that getRowAsArrayWi thStringIndex would.
*
* public
* returns mixed - the content of the field
*/
function getJustOneField () {
$field = $this->datastoreResul tsObject->getJustOneFiel d();
if ($field) {
$snip = substr($field, 0, 80);
$className = get_class($this->datastoreResul tsObject);
$this->resultsObjec t->selectNotes("I n the method getJustOneField ,
in the class McSelect, we ran a query with '$className' and what we
got back starts: $snip ...");
return $field;
} else {
$this->resultsObjec t->selectNotes("I n getJustOneField (), In the
class McSelect, we wanted to get some data back from the database, but
we did not. This isn't an error, because there are legitimate times
when no data will come back.");
}
}

}


?>

[Asgeir Frimannsson]

Re: need feed back about the comments I write

lawrence wrote:[color=blue]
> Below is a typical class I've written. Besides simply writing comments
> on all the methods, and besides keeping things up to date (I already
> know I should do these things) is there any other style tips or advice
> you'd like to offer? No ad homenien attacks, no flame wars please,
> just constructive criticism.[/color]

I would suggest using a documentation standard like phpDocumentor from
http://phpdocu.sourceforge.net. This is the standard used by PEAR and
many other libraries. But note, there are many other alternatives to
phpDocumentor, like Doxygen (http://www.docygen.org) and PHP Edit
(http://www.phpedit.net) (for windows only).

Using a tool like this, you can parse your php files and create a nice
html(or pdf or whatever) API documentation of your soure code.

Another tip is to follow a strict coding standard for your code. A good
place to start is PEAR's coding standard at

http://pear.php.net/manual/en/standards.php

Another tip, keep your lines max 80 chars long.

Also, i note you're using dates in your comment. You might consider
using a versioning system like cvs or Subversion. That would give you
more controll over your source code, being able to see exactly when you
edited a file, and what you edited.

regards,
asgeir

[lawrence]

Re: need feed back about the comments I write

Asgeir Frimannsson <a.frimannsson@ student.qut.edu .au> wrote in message news:<405f7433$ 0$16586$5a62ac2 2@freenews.iine t.net.au>...[color=blue]
> lawrence wrote:[color=green]
> > Below is a typical class I've written. Besides simply writing comments
> > on all the methods, and besides keeping things up to date (I already
> > know I should do these things) is there any other style tips or advice
> > you'd like to offer? No ad homenien attacks, no flame wars please,
> > just constructive criticism.[/color]
>
> I would suggest using a documentation standard like phpDocumentor from
> http://phpdocu.sourceforge.net. This is the standard used by PEAR and
> many other libraries. But note, there are many other alternatives to
> phpDocumentor, like Doxygen (http://www.docygen.org) and PHP Edit
> (http://www.phpedit.net) (for windows only).
>
> Using a tool like this, you can parse your php files and create a nice
> html(or pdf or whatever) API documentation of your soure code.
>
> Another tip is to follow a strict coding standard for your code. A good
> place to start is PEAR's coding standard at
> http://pear.php.net/manual/en/standards.php[/color]

Thank you, that was quite informative. I found this page quite useful:

http://phpdoc.org/docs/HTMLSmartyConverter/default/phpDocumentor/tutorial_phpDocumentor.pkg.html