Home » Structures » Structures_DataGrid » Bug #8090

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

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

After a code review of DataGrid, I have some questions on precise points. So I thought it would be simpler to put those questions in the source code. Please follow the link http://www.phpfi.com/126697 for a patch including the questions as PHP comments.

HTH
Michel

[2006-07-03 08:23 UTC] michel dot dhooge at gmail dot com

- Column.php:
> Constructors can't return anything. Would
> $this = PEAR::raiseError(...) be possible?
> Would this cause any problems somewhere else
> in the (SDG / user) code?
Not that easy, indeed ! I didn't realise we were in the constructor... Maybe the simplest is to put a warning in the comment: "If you aren't sure of the validity of the formatter callback, please call explicitely setFormatter()"

- Column.php, #2:
Well, _pCS() is really just a subpart of sF() but I agree smaller methods are easier to read.

[2006-07-03 10:47 UTC] olivierg at php dot net

Mark wrote :
> - Column.php:
> Constructors can't return anything.
> Would $this = PEAR::raiseError(...) be possible?

No..

> Would this cause any problems somewhere else in
> the (SDG / user) code?

Usually, in a constructor you just call PEAR::raiseError(), without returning anything. It works for debugging purpose, if PEAR error handling is set to PEAR_ERROR_PRINT, or the like.

Michel wrote :
> - Column.php, #2:
> Well, _pCS() is really just a subpart of sF() but I agree
> smallermethods are easier to read.

No, it is no sub-part. _parseCallbackString() mainly exists because it is called twice from setFormatter(). How would you do ? Duplicate the code inside setFormatter()?

Btw, let's please work on more serious things.

Mark wrote :
> - Renderer.php: Valid point, the formatters don't need
> to be called for renderers like Pager or HTMLSortForm.
> I'll add a new renderer property in the renderers and a
> switch before the for loop in Renderer.php.

Actually, it's a deeper problem. For example, with the Pager driver, the DataSource should only count the data, not fetch it. And for the HTMLSortForm driver, the data does not even need to be counted.

So the DataGrid class must know what the Renderer needs so that it can properly query the DataSource. In this regard, I think that a mechanism similar to the DataSource "features" (hasFeature(), setFeatures(), etc...) could be needed.

There could then be features like "renderLimits", "renderContent" (both true by default) and the DataGrid would "ask" with :
if ($renderer->hasFeature('renderLimits')) {count the rows}
if ($renderer->hasFeature('renderContent')) {fetch the rows}

The core renderer layer would of course also know about supported features and properly handle loops like the one you're talking about.

[2006-08-17 14:06 UTC] michel dot dhooge at gmail dot com

comment on SDG_DS_DBDO is in bug #8106