PEAR is archived and read-only

This mirror preserves historical PEAR package releases and metadata so existing references remain available.

Home » Structures » Structures_DataGrid » Bug #8089

Comment review

Details

Request #8089Comment review
Submitted2006-06-30 13:40 UTC
Frommichel dot dhooge at gmail dot com
Assignedwiesemann
StatusClosed
PackageStructures_DataGrid
PHP VersionIrrelevant
Roadmaps(Not assigned)

Comments

[2006-06-30 13:40 UTC] michel dot dhooge at gmail dot com

Description:
------------
Dear,

Here is a patch to improve the comments of DataGrid.php and DataSources.
http://www.phpfi.com/126693?lang=patch

Cheers
Michel

[2006-06-30 16:27 UTC] olivierg at php dot net

The DataSource's "field" option is not an "empty array which must be filled-in by the overloading classes". It is meant to be filled by the user, usually when calling bind(). Drivers may (not must) modify it transparently (as DBDO).

Why do you duplicate the "fields" option in the DataObject driver ? It is common to all DataSource drivers. Two different documentation entries, at different places, for the same option, is not a good idea.

But it looks like you want to say "okay, that's the field option, but with DBDO it behaves in a specific manner". Mark, as you're working on options doc, what do you think about this ?

By the way, to learn about our current way of documenting options, please look at HTMLTable.php

Apart from that, it looks good to me.

[2006-07-03 09:29 UTC] michel dot dhooge at gmail dot com

> The DataSource's "field" option is not an "empty
> array which must be filled-in by the overloading
> classes".
OK. My comment would have been more accurate like this:
It is empty when the object is created and could be automatically filled-in by some classes (see details of the corresponding class for details).

And BTW, the text "(Only used when "generate_columns" is true.)" is neither accurate: DataObject uses the fields options to know which variables must be read in the fetch() method...

> Why do you duplicate the "fields" option?
As you guessed, I wanted to show the specific behaviour of DBDO.

> I prefer the last variant, it makes it easy for
> people to see all options.
Is is possible to do this automatically with phpDocumentor?

[2006-07-04 14:36 UTC] olivierg at php dot net

Mark:
> About the options: We should not duplicate the comments
> for each driver, that will cause too much headaches for
> updates.

I'm not sure about this actually. It may be necessary to talk about the specific behaviour, side-effects, extra-informations of an option for a specific driver.

> i.e. first the table with the driver-specific options,
> then the table with the common options.

IMO, on the end-user doc, each driver should be associated with a list of options, that transparently contains both common and driver-specific options, all arranged in alphabetical order. A little sentence could mention if this is a common option or not, if necessary.

And don't forget that some driver may inherit from both DataSource and DataSource_Array (maybe more in the future).

Michel:
> And BTW, the text "(Only used when "generate_columns"
> is true.)" is neither accurate

Right. Fixed in CVS.

>> I prefer the last variant, it makes it easy for
>> people to see all options.
> Is is possible to do this automatically with phpDocumentor?

That's precisely what we're talking about. The problem is that in addition to phpDocumentor nice generation features, we also need to provide some sort of "options documentation inheritance", as discussed above.

Check tools/manual-gen in the CVS if you're curious about it.

[2006-07-04 16:01 UTC] olivierg at php dot net

100% agreed

Will your script be able to parse an undefined number of parents, grand-parents, etc ?

[2006-08-17 12:48 UTC] michel dot dhooge at gmail dot com

IMO, the only part that is still of interest if the
comment made by Mark on 2006-07-04 15:53 about
documentation inheritance.