Cleaner and more consistent code.
A while ago I suggested on d.o. to remove either db_fetch_row or db_fetch_object. This would result in cleaner code, more consistent function signatures and hooks. At my current work this is included in the coding conventions, we only use mysql_fetch_object(). I'm posting this to the development mailing because I am interested in what you, as developers, think about this. The original suggestion: http://drupal.org/node/158115 Example of an inconsistent hook: http://drupal.org/node/124141
I would suggest to make Drupal core use only db_fetch_object but to keep support for db_fetch_row for contributed modules. In core it is very confusing to see the following in node.module for instance: function node_view($node, $teaser = FALSE, $page = FALSE, $links = TRUE) { $node = (object)$node; function node_submit($node) { global $user; // Convert the node to an object, if necessary. $node = (object)$node; function node_validate($node, $form = array()) { // Convert the node to an object, if necessary. $node = (object)$node; function node_access($op, $node = NULL) { global $user; // Convert the node to an object if necessary: if ($op != 'create') { $node = (object)$node; } It would lead to more consistent code to make it best practice to always pass a node as an object for instance. Wim ojacquet@jax.be wrote:
A while ago I suggested on d.o. to remove either db_fetch_row or db_fetch_object. This would result in cleaner code, more consistent function signatures and hooks. At my current work this is included in the coding conventions, we only use mysql_fetch_object(). I'm posting this to the development mailing because I am interested in what you, as developers, think about this.
The original suggestion: http://drupal.org/node/158115
Example of an inconsistent hook: http://drupal.org/node/124141
I personally use objects, but arrays are more feature rich, e.g. you can merge arrays, enumerate keys, and so on. But since Drupal 7 was announced to require PHP5, I would rather postpone this decision until we know in which direction the core code base will evolve. Either PDO will be available to modules, in which case there's no difference, or maybe (just maybe) another data abstraction layer will be introduced, with node_* hooks being replaced with interface implementations, then it will most likely be objects. -- hex, out.
On 8/2/07, ojacquet@jax.be <ojacquet@jax.be> wrote:
A while ago I suggested on d.o. to remove either db_fetch_row or db_fetch_object.
I'm assuming you mean db_fetch_array()... seeing as there's no db_fetch_row(): http://api.drupal.org/api/search/6/db_fetch_row
Considering that the "objects" we use aren't objects (in the sense of having member functions) and arrays are more capable structures, I would rather standardize on arrays. ojacquet@jax.be wrote:
A while ago I suggested on d.o. to remove either db_fetch_row or db_fetch_object. This would result in cleaner code, more consistent function signatures and hooks. At my current work this is included in the coding conventions, we only use mysql_fetch_object(). I'm posting this to the development mailing because I am interested in what you, as developers, think about this.
The original suggestion: http://drupal.org/node/158115
Example of an inconsistent hook: http://drupal.org/node/124141
I'm with David - especially using Forms API - getting data in and out with arrays is more consistent. If I *had* to chose one, I'd choose db_fetch_array(). -Peter On 8/2/07, David Strauss <david@fourkitchens.com> wrote:
Considering that the "objects" we use aren't objects (in the sense of having member functions) and arrays are more capable structures, I would rather standardize on arrays.
The original suggestion: http://drupal.org/node/158115
Example of an inconsistent hook: http://drupal.org/node/124141
I forget where it was posted, but I believe Dries recently posted a count that says core uses like 80% db_fetch_object() over db_fetch_array(). The move to PHP 5 in Drupal 7, though changes the equation. Objects become more than just alternate array syntax, and we can leverage objects as objects if it makes sense to. (I certainly see advantages to nodes becoming real objects rather than arrays that sometimes use ->.) That makes the "arrays are more flexible than objects" argument moot, because that's only a sane comparison to make when you're dealing with stdClass objects. I think it's premature now to say that we ought to get rid of objects in favor of arrays, just when we're finally getting objects that aren't limited to being arrays. The other factor is PDO. I am planning to move the database layer to a PDO[1] backend in Drupal 7[2]. When that happens, db_fetch_*() will cease to exist. Instead, db_query() and company will return a PDOStatement result object. That result object will have a ->fetch()[3] method, which will always return either an object or an array, depending on the settings applied to the connection. It will be overridable with a parameter in case you really want the other. At present I am planning to default to object, since that's what core uses most. (There will also be other ->fetch*() methods, both those provided by PDO and additional ones added in a sublcass, but I'm still working on the details there.) [1] http://us3.php.net/pdo [2] http://www.garfieldtech.com/blog/drupal-7-database-plans [3] http://us3.php.net/manual/en/function.PDOStatement-fetch.php --Larry Garfield On Thu, 2 Aug 2007 12:27:29 +0200, <ojacquet@jax.be> wrote:
A while ago I suggested on d.o. to remove either db_fetch_row or db_fetch_object. This would result in cleaner code, more consistent function signatures and hooks. At my current work this is included in the coding conventions, we only use mysql_fetch_object(). I'm posting this to the development mailing because I am interested in what you, as developers, think about this.
The original suggestion: http://drupal.org/node/158115
Example of an inconsistent hook: http://drupal.org/node/124141
After reading Larry's blog post and some of the links referred there it seems there's a strong movement for using PDO in D7 which would render the whole point moot. So the only question left is to see if there's support for refactoring the core of drupal 6 to one of the two functions. And if there is, which? Is there even time to do this?
On 8/2/07, ojacquet@jax.be <ojacquet@jax.be> wrote:
After reading Larry's blog post and some of the links referred there it seems there's a strong movement for using PDO in D7 which would render the whole point moot. So the only question left is to see if there's support for refactoring the core of drupal 6 to one of the two functions. And if there is, which? Is there even time to do this?
I'd be strongly against that type of a change. Doing so would touch just about every file in core and contrib, and probably introducing bugs in the process. I'd want to see real big gain for that much effort and at this point I don't. andrew
I think right now this would require substantial effort for zero actual gain other than aesthetics, and we'd be chasing the resulting bugs for weeks. -Peter On 8/2/07, ojacquet@jax.be <ojacquet@jax.be> wrote:
After reading Larry's blog post and some of the links referred there it seems there's a strong movement for using PDO in D7 which would render the whole point moot. So the only question left is to see if there's support for refactoring the core of drupal 6 to one of the two functions. And if there is, which? Is there even time to do this?
On 2-Aug-07, at 12:09 PM, <ojacquet@jax.be> <ojacquet@jax.be> wrote:
After reading Larry's blog post and some of the links referred there it seems there's a strong movement for using PDO in D7 which would render the whole point moot. So the only question left is to see if there's support for refactoring the core of drupal 6 to one of the two functions. And if there is, which? Is there even time to do this?
We're post-code freeze, which means no API changes unless they fix critical bugs, or were previously approved for inclusion (some performance patches, etc.). So no, this won't happen for Drupal 6. -Angie
Karoly (chx) seems to be in favor of going all arrays and getting rid of pseudo objects. http://drupal4hu.com/node/52 He mentions others who support this too. Drupal 7 is the time to do this, not D6.
I strongly support this move. Khalid Baheyeldin wrote:
Karoly (chx) seems to be in favor of going all arrays and getting rid of pseudo objects.
He mentions others who support this too.
Drupal 7 is the time to do this, not D6.
On 03 Aug 2007, at 11:54 AM, David Strauss wrote:
I strongly support this move.
Khalid Baheyeldin wrote:
Karoly (chx) seems to be in favor of going all arrays and getting rid of pseudo objects.
He mentions others who support this too.
Drupal 7 is the time to do this, not D6.
I believe in this too, as it melds more with Drupal's structured array syntax, but PHP5 might change my opinion. Especially when you throw in things like the ArrayObject syntax (array / object duality), which could even potentially supplant #-notation. So yeah. needs more time to gestate methinks. <?php class form extends ArrayObject { function __construct($props = null) { foreach ((array) $props as $key => $value) { $this->{$key} = $value; } } } class element extends ArrayObject { function __construct($props = null) { foreach ((array) $props as $key => $value) { $this->{$key} = $value; } } } $form = new form(); $form['title'] = new element(array( // this would probably be new DrupalTextfield() or something 'type' => 'textfield', 'title' => 'my title', 'default_value' => 'default text' )); $form['group'] = new element(array( 'type' => 'fieldset', 'title' => 'meh') ); $form['group']['moretext'] = new element(array( 'type' => 'textfield', 'title' => 'blah' )); print $form['group']->type; print $form['title']->default_value; ~
On Aug 3, 2007, at 7:10 AM, adrian rossouw wrote:
$form = new form(); $form['title'] = new element(array( // this would probably be new DrupalTextfield() or something 'type' => 'textfield', 'title' => 'my title', 'default_value' => 'default text' )); $form['group'] = new element(array( 'type' => 'fieldset', 'title' => 'meh') );
$form['group']['moretext'] = new element(array( 'type' => 'textfield', 'title' => 'blah' ));
print $form['group']->type; print $form['title']->default_value;
You just made me the happiest boy in the world. Honestly, this straightforward nesting syntax is the only reason that I've felt we should steer away from objects in D6. the use of # to mean 'property' took a log of explaining. Getting rid of that, while maintaining the clean nesting syntax, would be a huge huge win.
On 03 Aug 2007, at 2:28 PM, Jeff Eaton wrote:
Honestly, this straightforward nesting syntax is the only reason that I've felt we should steer away from objects in D6. the use of # to mean 'property' took a log of explaining. Getting rid of that, while maintaining the clean nesting syntax, would be a huge huge win.
Next, I really wish that somewhere down the road php gets named parameters. That would be the fscking awesome. So instead of doing : $form['title'] = new drupal_textfield(array('title' => 'blah', 'default_value' => blah)); we could do : $form['title'] = new drupal_textfield(title = 'blah', default_value = 'blah');
On Friday 03 August 2007, adrian rossouw wrote:
On 03 Aug 2007, at 2:28 PM, Jeff Eaton wrote:
Honestly, this straightforward nesting syntax is the only reason that I've felt we should steer away from objects in D6. the use of # to mean 'property' took a log of explaining. Getting rid of that, while maintaining the clean nesting syntax, would be a huge huge win.
Next, I really wish that somewhere down the road php gets named parameters. That would be the fscking awesome. So instead of doing :
$form['title'] = new drupal_textfield(array('title' => 'blah', 'default_value' => blah));
we could do :
$form['title'] = new drupal_textfield(title = 'blah', default_value = 'blah');
Bug the php-internals folks. ;-) One thing that I've been considering lately is that FAPI structure are right now about 90% compatible with SimpleXML. SimpleXML uses ArrayObject, and child objects are referenced with -> while properties are referenced with []. Then you get all sorts of iteration magic with it, the ability to trivially dump to/from XML, etc. That other 10%, though, is array properties. #options is an array, by nature, and therefore can't easily serialize to XML. I don't know if there's a good solution for that. That said, a syntax that somewhat parallels SimpleXML and leverages ArrayObject, iterators, and the rest of SPL, but just doesn't translate directly to XML the way SimpleXML does, could make FAPI a lot cleaner and more intuitive. Oh, so many options... :-) (Go vote for the "Drupal and PHP5" Barcelona presentation that chx and I want to give so we can discuss this in person!) -- Larry Garfield AIM: LOLG42 larry@garfieldtech.com ICQ: 6817012 "If nature has made any one thing less susceptible than all others of exclusive property, it is the action of the thinking power called an idea, which an individual may exclusively possess as long as he keeps it to himself; but the moment it is divulged, it forces itself into the possession of every one, and the receiver cannot dispossess himself of it." -- Thomas Jefferson
On 03 Aug 2007, at 4:58 PM, Larry Garfield wrote:
That said, a syntax that somewhat parallels SimpleXML and leverages ArrayObject, iterators, and the rest of SPL, but just doesn't translate directly to XML the way SimpleXML does, could make FAPI a lot cleaner and more intuitive.
Well. I found the issue I have with simplexml, namely that you can't serialize the object. Basically what it comes down to is we need to implement the __sleep() and __wakeup() functions to properly handle serializing anything. There's also a couple of other gotcha's, but I'm confident they can be worked around.
participants (11)
-
adrian rossouw -
andrew morton -
Angela Byron -
David Strauss -
hex -
Jeff Eaton -
Khalid Baheyeldin -
Larry Garfield -
ojacquet＠jax.be -
Peter Wolanin -
Wim Mostrey