By using this site, you agree to our updated Privacy Policy and our Terms of Use. Manage your Cookies Settings.
424,853 Members | 936 Online
Bytes IT Community
+ Ask a Question
Need help? Post your question and get tips & solutions from a community of 424,853 IT Pros & Developers. It's quick & easy.

need feed back about the comments I write

P: n/a
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 $controllerForAll = null;
var $resultsObject = null;
// 03-06-04 - the 2 worker objects
var $queryObject = null;
var $datastoreResultsObject = 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 $secretQueryObjectCheckCode;



/**
* 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
McDatastoreResults.
*
* 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.
*
* $datastoreResultsObject - 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 $getterDataObject
*
* 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
showRecentWeblogEntries2):
*
* global $controllerForAll;
* $selectObject = & $controllerForAll->getObject("McSelect", " in
showRecentWeblogEntries2().");
*
* $selectObject->setDatastoreObject();
* $selectObject->setQueryObject("GetAllOfTypeReversed");
* $selectObject->setInfoToBeSought("weblogEntries");
* $selectObject->getInfo();
*
* $howMany = $selectObject->getCountOfReturn();
*
* The first method, setDatastoreObject, 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, setInfoToBeSought, 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, setInfoToBeSought("keywords"), and instead
favor GetAllKeywords.
*
* The function getCountOfReturn() allows you to use the selectObject
from inside of a for loop, like here:
*
* for ($i=0; $i < $howMany; $i++) {
* $row = $selectObject->getRowAsArrayWithStringIndex();
* if (is_array($row)) extract($row);
* print $cbHeadline;
* }
*
* public
* returns void
*/
function McSelect() {
$this->controllerForAll = & $GLOBALS["controllerForAll"];
$this->resultsObject = &
$this->controllerForAll->getObject("McResults", " 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?
* getRecentEntries()? printSelectToScreen() 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 "getRecentEntries". If a class method,
"printSelectToScreen in McFormsNavigation".
*
* public
* returns void
*/
function begin($message=false) {
if ($message) {
$this->resultsObject->selectNotes($message);
} else {
$this->resultsObject->addToErrorMessages("In begin(), in McSelect,
we expected a message to be given to us, but there was none.");
}
}


/**
* 03-06-04 - setter of McQueryObject subclass
*
* $queryObjectNameSuffix and $queryObject are two strings which get
combined to specify which subclass of McQuery should get loaded.
* $queryObjectNameSuffix contains the datastore in use (like "MySql"
or "Xml"), where as $queryObject merely contains the query
("GetAllOfType").
* $controllerForAll 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 - $queryObjectNameSuffix - 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=false,
$queryObjectNameSuffix=false) {
if (is_string($queryObject)) {
if (!$queryObjectNameSuffix) {
// 03-06-04 - normally $queryObjectNameSuffix 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=getConfig();
$queryObjectNameSuffix = $config["defaultQueryObjectPrefix"];
}
$query = "McQueryObject".$queryObjectNameSuffix.$queryObjec t;
$this->queryObject = & $this->controllerForAll->getObject($query, "
setQueryObject(), inside of McSelect");

if (is_object($this->queryObject)) {
$this->queryObject->setDatastoreConnector($queryObjectNameSuffix);

$className = get_class($this->queryObject);
$this->resultsObject->selectNotes("In 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 '
$queryObjectNameSuffix ' prefix.", "McSelect");
} else {
$this->resultsObject->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->resultsObject->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($queryObject) {
// 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 setInfoToBeSought($infoToBeSought1=false,
$infoToBeSought2=false, $infoToBeSought3=false) {
$this->queryObject->setInfoToBeSought($infoToBeSought1,
$infoToBeSought2, $infoToBeSought3);

$className = get_class($this->queryObject);
$this->resultsObject->selectNotes("In setInfoToBeSought in the class
McSelect this is our first value: $infoToBeSought1 and this is out
second: $infoToBeSought2 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 getRowAsArrayWithStringIndex(),
* getResultAsArrayWithStringIndex(), 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.
$dsResultPointer = $this->queryObject->getInfo();

if (is_resource($dsResultPointer)) {
$className = get_class($this->queryObject);
$this->resultsObject->selectNotes("In 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->datastoreResultsObject->setInfoResourcePointer($dsResultPointer);
} else {
$className = get_class($this->queryObject);
$query = $this->queryObject->getQuery();
$this->resultsObject->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
McDatastoreResultsFile would then be able to get data out of that
pointer.
*
* @param - $objectSuffix - optional - if present, this is what we
attach to the string "McDatastoreResults" and then
* try to find a class by that name. A common example might be "File"
which then becomes "McDatastoreResultsFile".
* This would be for getting data from a file pointer. "MySql" would
lead to "McDatastoreResultsMySql" and could get
* data back from database return.
*
* public
* returns void
*/
function setDatastoreObject($objectSuffix=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->datastoreResultsObject)
unset($this->datastoreResultsObject);

if (is_string($objectSuffix)) {
$name = "McDatastoreResults".$objectSuffix;
$this->datastoreResultsObject = &
$this->controllerForAll->getObject($name, " in setDatastoreObject() of
McSelect.");
} else {
$config = getConfig();
$objectSuffix = $config["defaultQueryObjectPrefix"];
$name = "McDatastoreResults".$objectSuffix;
$this->datastoreResultsObject = &
$this->controllerForAll->getObject($name, " in setDatastoreObject() of
McSelect.");
}
}


/**
* 11-08-03 - We cannot call getRowAsArrayWithStringIndex() unless we
know how many items there are in the return.
* Actually, there are other ways, but this is one.
*/
function getCountOfReturn() {
$count = $this->datastoreResultsObject->getCountOfReturn();
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 getRowAsArrayWithStringIndex() {
if (is_object($this->datastoreResultsObject)) {
$row = $this->datastoreResultsObject->getRowAsArrayWithStringIndex();

$className = get_class($this->datastoreResultsObject);
if (is_array($row)) {
$this->resultsObject->selectNotes("In
getRowAsArrayWithStringIndex(), 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->resultsObject->error("In getRowAsArrayWithStringIndex(), in
the class McSelect, we'd like to get some data from the datastore
results object, but there is no such object. Was setDatastoreObject()
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 $formatDatastoreResults.
* 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 getResultAsArrayWithStringIndex() {
if (is_object($this->datastoreResultsObject)) {
$dsArrayWithStringIndex =
$this->datastoreResultsObject->getResultAsArrayWithStringIndex();
return $dsArrayWithStringIndex;
} else {
$this->resultsObject->error("In getResultAsArrayWithStringIndex(),
in the class McSelect, we wanted to get some data from the datastore
results object, but it didn't exist. Was setDatastoreObject()
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 getRowAsArrayWithStringIndex would.
*
* public
* returns mixed - the content of the field
*/
function getJustOneField() {
$field = $this->datastoreResultsObject->getJustOneField();
if ($field) {
$snip = substr($field, 0, 80);
$className = get_class($this->datastoreResultsObject);
$this->resultsObject->selectNotes("In the method getJustOneField,
in the class McSelect, we ran a query with '$className' and what we
got back starts: $snip ...");
return $field;
} else {
$this->resultsObject->selectNotes("In 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.");
}
}

}
?>
Jul 17 '05 #1
Share this Question
Share on Google+
2 Replies


P: n/a
lawrence wrote:
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.


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
Jul 17 '05 #2

P: n/a
Asgeir Frimannsson <a.***********@student.qut.edu.au> wrote in message news:<40***********************@freenews.iinet.net .au>...
lawrence wrote:
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.


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


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

http://phpdoc.org/docs/HTMLSmartyCon...entor.pkg.html
Jul 17 '05 #3

This discussion thread is closed

Replies have been disabled for this discussion.