need a standard for contrib node build_mode constants
When doing some cleanup of my Modr8 module, I wanted to define a new build_mode for use by http://api.drupal.org/api/function/node_build_content/6 I realized that we don't have any standard for picking new int constants for this case in specific, or more generally. So, here's a suggested standard that I think is nearly fool-proof: When defining an additional build mode constant (or similar constant) in a contributed module, the suggested standard is to use the unix timestamp of when you write the code to minimize the likelihood of two modules using the same value. I think it's sane and nearly perfect since a new module will then never pick the same value as an existing module, and in general it's very unlikely that 2 people will by writing a new constant the same second. Am I missing anything? Any reason not to suggest this as a standard? -Peter PS - a couple easy ways to get the timestamp on you command line: date -j "+%s" php -r 'echo time() ."\n";'
Peter Wolanin wrote:
When doing some cleanup of my Modr8 module, I wanted to define a new build_mode for use by http://api.drupal.org/api/function/node_build_content/6
I believe there is no need to stick with ints; CCK uses this and I think it's using strings.
The reason I was suggesting sticking with ints is that strings are cast to int 0 during comparison: php -r"var_dump('cck' == 0);" bool(true) And core has code like: modules/upload/upload.module:363: if ($node->build_mode == NODE_BUILD_RSS) { modules/book/book.module:710: if (!empty($node->book['bid']) && $node->build_mode == NODE_BUILD_NORMAL) { So if CCK is using ints, that's a potentially serious bug - 0 is NODE_BUILD_NORMAL, so I think any string build modes will basically end up being this mode. -Peter On Sat, May 16, 2009 at 11:50 PM, Earl Miles <merlin@logrus.com> wrote:
Peter Wolanin wrote:
When doing some cleanup of my Modr8 module, I wanted to define a new build_mode for use by http://api.drupal.org/api/function/node_build_content/6
I believe there is no need to stick with ints; CCK uses this and I think it's using strings.
Peter's suggestion may work for D6, but for D7, I refer people to Eaton's "How to make good APIs" session from DCDC. Digest version: If you expect people to extend your list of flags/constants/modes, for the love of god use strings, not ints. Ints WILL break. Don't be stupid, use strings. So it sounds like if we expect people to extend the build modes from contrib, then core should switch from ints to strings anyway. Who wants to roll the patch? :-) --Larry Garfield Peter Wolanin wrote:
The reason I was suggesting sticking with ints is that strings are cast to int 0 during comparison:
php -r"var_dump('cck' == 0);"
bool(true)
And core has code like:
modules/upload/upload.module:363: if ($node->build_mode == NODE_BUILD_RSS) {
modules/book/book.module:710: if (!empty($node->book['bid']) && $node->build_mode == NODE_BUILD_NORMAL) {
So if CCK is using ints, that's a potentially serious bug - 0 is NODE_BUILD_NORMAL, so I think any string build modes will basically end up being this mode.
-Peter
On Sat, May 16, 2009 at 11:50 PM, Earl Miles <merlin@logrus.com> wrote:
Peter Wolanin wrote:
When doing some cleanup of my Modr8 module, I wanted to define a new build_mode for use by http://api.drupal.org/api/function/node_build_content/6 I believe there is no need to stick with ints; CCK uses this and I think it's using strings.
Peter clarified in IRC that his timestamp proposal for for D6 and that we should use strings for D7. And yes, we need a patch at http://drupal.org/node/409750 On Mon, May 18, 2009 at 3:13 PM, larry@garfieldtech.com <larry@garfieldtech.com> wrote:
Peter's suggestion may work for D6, but for D7, I refer people to Eaton's "How to make good APIs" session from DCDC. Digest version: If you expect people to extend your list of flags/constants/modes, for the love of god use strings, not ints. Ints WILL break. Don't be stupid, use strings.
So it sounds like if we expect people to extend the build modes from contrib, then core should switch from ints to strings anyway. Who wants to roll the patch? :-)
--Larry Garfield
Peter Wolanin wrote:
The reason I was suggesting sticking with ints is that strings are cast to int 0 during comparison:
php -r"var_dump('cck' == 0);"
bool(true)
And core has code like:
modules/upload/upload.module:363: if ($node->build_mode == NODE_BUILD_RSS) {
modules/book/book.module:710: if (!empty($node->book['bid']) && $node->build_mode == NODE_BUILD_NORMAL) {
So if CCK is using ints, that's a potentially serious bug - 0 is NODE_BUILD_NORMAL, so I think any string build modes will basically end up being this mode.
-Peter
On Sat, May 16, 2009 at 11:50 PM, Earl Miles <merlin@logrus.com> wrote:
Peter Wolanin wrote:
When doing some cleanup of my Modr8 module, I wanted to define a new build_mode for use by http://api.drupal.org/api/function/node_build_content/6
I believe there is no need to stick with ints; CCK uses this and I think it's using strings.
On Mon, May 18, 2009 at 12:13 PM, larry@garfieldtech.com <larry@garfieldtech.com> wrote:
Peter's suggestion may work for D6, but for D7, I refer people to Eaton's "How to make good APIs" session from DCDC. Digest version: If you expect people to extend your list of flags/constants/modes, for the love of god use strings, not ints. Ints WILL break. Don't be stupid, use strings.
Hummm. Ints, strings, these are just ways to express bits of information. Each character of a string will have approx 5 bit of info (or a bit less, 26 english chars, underscore, thats 27 not 32 but lets say 32). It also must be noted that its unlikely you will use all possible combinations, I bet XRNQW will not be used for example... I am with Peter, his proposal makes sense to me and can code more possibilities than strings.
The problem is not one of information storage length, but of flag collision. Ints are fine as primary keys on a lookup table, or constant values for internal flags that will never be extensible. For things we know people will build on -- like node types, or node build modes -- strings are the only way to avoid collisions when module developers start expanding. --Jeff On May 18, 2009, at 3:27 PM, Karoly Negyesi wrote:
Ints, strings, these are just ways to express bits of information.
Each character of a string will have approx 5 bit of info (or a bit less, 26 english chars, underscore, thats 27 not 32 but lets say 32). It also must be noted that its unlikely you will use all possible combinations, I bet XRNQW will not be used for example...
I am with Peter, his proposal makes sense to me and can code more possibilities than strings.
To the contrary, Peter's proposition makes a very good standard to avoid collisions, it's actually better than strings IMO.
Karoly Negyesi wrote:
To the contrary, Peter's proposition makes a very good standard to avoid collisions, it's actually better than strings IMO.
Hardly. You have an int that has no real meaning and then a string anyway, which is the define. The *only* improvement is that in theory a 32 bit int is slightly faster to look up via index.
To the contrary, Peter's proposition makes a very good standard to avoid collisions, it's actually better than strings IMO.
To really make it unique, you'd want to prepend your drupal.org uid and append your shoe size.
Quoting Jeff Eaton <jeff@viapositiva.net>:
The problem is not one of information storage length, but of flag collision.
Ints are fine as primary keys on a lookup table, or constant values for internal flags that will never be extensible. For things we know people will build on -- like node types, or node build modes -- strings are the only way to avoid collisions when module developers start expanding.
The only way to avoid a collision between developers of modules is to create a namespace based on the module name for whatever the data is. So for node_type a unique key of module and name creates the necessary uniqueness required. The system though needs to include the module for the content type in it's presentation so that one knows by looking at the UI which module the content type is for. Perhaps the use of fieldsets where the fieldset is the module name could help clear up the confusion when more than one module could create a UI data conflict. -- Earnie -- http://r-feed.com/ -- http://for-my-kids.com/ -- http://www.4offer.biz/ -- http://give-me-an-offer.com/
Indeed. At least with strings it's possible to develop meaningful conventions. With ints, there is literally no meaning other than "first come, first served, set up a registry." --Jeff On May 19, 2009, at 1:57 PM, Earnie Boyd wrote:
Quoting Jeff Eaton <jeff@viapositiva.net>:
The problem is not one of information storage length, but of flag collision.
Ints are fine as primary keys on a lookup table, or constant values for internal flags that will never be extensible. For things we know people will build on -- like node types, or node build modes -- strings are the only way to avoid collisions when module developers start expanding.
The only way to avoid a collision between developers of modules is to create a namespace based on the module name for whatever the data is. So for node_type a unique key of module and name creates the necessary uniqueness required. The system though needs to include the module for the content type in it's presentation so that one knows by looking at the UI which module the content type is for. Perhaps the use of fieldsets where the fieldset is the module name could help clear up the confusion when more than one module could create a UI data conflict.
After all, we wouldn't want developers to have actually document anything they write. ;-) On Wed, May 20, 2009 at 1:04 PM, Jeff Eaton <jeff@viapositiva.net> wrote:
Indeed.
At least with strings it's possible to develop meaningful conventions. With ints, there is literally no meaning other than "first come, first served, set up a registry."
--Jeff
On May 19, 2009, at 1:57 PM, Earnie Boyd wrote:
Quoting Jeff Eaton <jeff@viapositiva.net>:
The problem is not one of information storage length, but of flag collision.
Ints are fine as primary keys on a lookup table, or constant values for internal flags that will never be extensible. For things we know people will build on -- like node types, or node build modes -- strings are the only way to avoid collisions when module developers start expanding.
The only way to avoid a collision between developers of modules is to create a namespace based on the module name for whatever the data is. So for node_type a unique key of module and name creates the necessary uniqueness required. The system though needs to include the module for the content type in it's presentation so that one knows by looking at the UI which module the content type is for. Perhaps the use of fieldsets where the fieldset is the module name could help clear up the confusion when more than one module could create a UI data conflict.
Quoting Chris Johnson <cxjohnson@gmail.com>:
After all, we wouldn't want developers to have actually document anything they write. ;-)
It isn't the developer of the module that would be confused. It would be the users of the module that would be confused and users rarely read the documentation because the developer is too technical with it. User initial experience of a product gears toward is the product self documenting or in other words natural to use. -- Earnie -- http://r-feed.com/ -- http://for-my-kids.com/ -- http://www.4offer.biz/ -- http://give-me-an-offer.com/
On May 21, 2009, at 11:50 AM, Chris Johnson wrote:
After all, we wouldn't want developers to have actually document anything they write. ;-)
Well... documentation is needed in either case. But with ints, collisions are absolutely inevitable. I ran into this with VotingAPI early on -- I had a const for 'vote_type', and it had 3 values out of the box. Pretty quickly, two other modules both defined fourth value types. suddenly, those modules couldn't co-exist with each other. There's nothing in *my* documentation that could have solved that, other than a pointer to a wiki page where everyone claimed ints.
I think the best example of using strings over ints is DNS. How many people know IP address of their favorite website? Using a numeric based name space is great for machines, but in order for the web to be usable, we had to create DNS. I suspect the same logic would apply here. If we are going to have a usable api, we should use a string. Mike O. With that in mind, you would have to do a lot to convince me that int's were a better solution for an API. On Thu, May 21, 2009 at 3:09 PM, Jeff Eaton <jeff@viapositiva.net> wrote:
On May 21, 2009, at 11:50 AM, Chris Johnson wrote:
After all, we wouldn't want developers to have actually document
anything they write. ;-)
Well... documentation is needed in either case. But with ints, collisions are absolutely inevitable. I ran into this with VotingAPI early on -- I had a const for 'vote_type', and it had 3 values out of the box. Pretty quickly, two other modules both defined fourth value types. suddenly, those modules couldn't co-exist with each other.
There's nothing in *my* documentation that could have solved that, other than a pointer to a wiki page where everyone claimed ints.
On Thu, May 21, 2009 at 2:09 PM, Jeff Eaton <jeff@viapositiva.net> wrote:
On May 21, 2009, at 11:50 AM, Chris Johnson wrote:
After all, we wouldn't want developers to have actually document anything they write. ;-)
Well... documentation is needed in either case. But with ints, collisions are absolutely inevitable. I ran into this with VotingAPI early on -- I had a const for 'vote_type', and it had 3 values out of the box. Pretty quickly, two other modules both defined fourth value types. suddenly, those modules couldn't co-exist with each other.
There's nothing in *my* documentation that could have solved that, other than a pointer to a wiki page where everyone claimed ints.
Thanks for the clear example, Jeff. It makes a huge difference. If developers of other modules can arbitrarily define new values your module has to work with, collisions are inevitable, whether integer or strings. Strings do enjoy some advantages, including being easier to grep for. P.S. Of course, *user* visible constants should be strings. I always thought we were talking about developer-only constants here.
Quoting Chris Johnson <cxjohnson@gmail.com>:
P.S. Of course, *user* visible constants should be strings. I always thought we were talking about developer-only constants here.
Which must be translated somehow for ease of use by the developer to strings. I.E. in PHP define ('DEVELOPER_ONLY_CONSTANTS', 1); -- Earnie -- http://r-feed.com/ -- http://for-my-kids.com/ -- http://www.4offer.biz/ -- http://give-me-an-offer.com/
participants (10)
-
Chris Johnson -
Earl Miles -
Earnie Boyd -
Jeff Eaton -
John VanDyk -
Karoly Negyesi -
larry＠garfieldtech.com -
Mike O'Connor -
Moshe Weitzman -
Peter Wolanin