[drupal-devel] PHP 101: isset() does not check for zero length (empty) strings
I literally spent hours trying to track this down (running Drupal 4.6.3). Please take a moment to review my findings. I was having a problem with the bug at http://drupal.org/node/20795 (I beg to differ that it is fixed as reported, by the way) on a couple of different feeds. The problem went away when I changed the following line in node_invoke_nodeapi() from: else if (isset($result)) { to this: else if (isset($result) && $result != '') { The problem, of course, was is that isset() returns true on an empty string. This caused an undesirable result in this particular instance. I've seen this bug elsewhere in Drupal's code. Should we be going through Drupal's code and fixing this problem which is probably pretty common? -- Dondley Communications http://www.dondleycommunications.com Communicate or Die: American Labor Unions and the Internet http://www.communicateordie.com
Awhile back I posted a similar question, see below. We have a need for some standard guidelines on when to use which test for empty, null, or set variables, arrays, and object properties. _______ I find I'm sometimes not sure which types of test I should apply to test if a variable (or array key) is null or empty. Looking at the code, I see various usages: if (is_null($variable)) if (empty($variable)) if ($variable == NULL) if ($variable === NULL) if ($objectname->propertyname) if (array_key_exists($key, $arrayname)) if ($arrayname['key']) etc. Can anyone point to or suggest a limited set of tests that we should use, and guidelines for when each should be applied? This would be a useful addition to the coding standards, http://drupal.org/node/318. Thanks!
/me blushes and admits the same fuzziness. Nedjo Rogers wrote:
Awhile back I posted a similar question, see below. We have a need for some standard guidelines on when to use which test for empty, null, or set variables, arrays, and object properties. _______
I find I'm sometimes not sure which types of test I should apply to test if a variable (or array key) is null or empty.
Looking at the code, I see various usages:
if (is_null($variable)) if (empty($variable)) if ($variable == NULL) if ($variable === NULL) if ($objectname->propertyname) if (array_key_exists($key, $arrayname)) if ($arrayname['key']) etc.
Can anyone point to or suggest a limited set of tests that we should use, and guidelines for when each should be applied? This would be a useful addition to the coding standards, http://drupal.org/node/318. Thanks!
+1 on having some recommendations on how to handle these, especially if we want to make Drupal E_STRICT compliant. (I do, anyway. <g>) Is there some PHP best practices guideline regarding this? Does PEAR have a recommended standard that we could borrow? On Friday 14 October 2005 02:04 am, Robert Douglass wrote:
/me blushes and admits the same fuzziness.
Nedjo Rogers wrote:
Awhile back I posted a similar question, see below. We have a need for some standard guidelines on when to use which test for empty, null, or set variables, arrays, and object properties. _______
I find I'm sometimes not sure which types of test I should apply to test if a variable (or array key) is null or empty.
Looking at the code, I see various usages:
if (is_null($variable)) if (empty($variable)) if ($variable == NULL) if ($variable === NULL) if ($objectname->propertyname) if (array_key_exists($key, $arrayname)) if ($arrayname['key']) etc.
Can anyone point to or suggest a limited set of tests that we should use, and guidelines for when each should be applied? This would be a useful addition to the coding standards, http://drupal.org/node/318. Thanks!
-- 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 Fri, 14 Oct 2005 08:26:08 +0200, Larry Garfield <larry@garfieldtech.com> wrote:
+1 on having some recommendations on how to handle these, especially if we want to make Drupal E_STRICT compliant. (I do, anyway. <g>)
Well. It is well known that these are not the same. http://www.php.net/manual/en/types.comparisons.php We have already found that is_null is slower but equals to === NULL so use the latter. In the table referenced above !isset always equals to is_null, so I propose to use !isset when we want to check for, well, not being set. if ($foo) and if (isset($foo)) is not the same, often it's a shorthand for if(isset($foo) && $foo != 0). I think if(isset($foo) && $foo) will always do. Sometimes isset() can be ommited because we know $foo is set. Regards NK
It is well known that these are not the same.
These are GREAT charts. Duly bookmarked.
We have already found that is_null is slower but equals to === NULL so use the latter. In the table referenced above !isset always equals to is_null, so I propose to use !isset when we want to check for, well, not being set.
if ($foo) and if (isset($foo)) is not the same, often it's a shorthand for if(isset($foo) && $foo != 0). I think if(isset($foo) && $foo) will always do. Sometimes isset() can be ommited because we know $foo is set.
Regards
NK
-- Dondley Communications http://www.dondleycommunications.com Communicate or Die: American Labor Unions and the Internet http://www.communicateordie.com
Oh, and also, loose array_key_exists. isset is a language construct thus faster.
I was thinking, this problem also stems from the fact that sometimes fields have a NULL value in the database and sometimes are stored as empty strings. We will probably want to settle on a way of storing string in the database, too. On 10/13/05, Nedjo Rogers <nedjo@gworks.ca> wrote:
Awhile back I posted a similar question, see below. We have a need for some standard guidelines on when to use which test for empty, null, or set variables, arrays, and object properties. _______
I find I'm sometimes not sure which types of test I should apply to test if a variable (or array key) is null or empty.
Looking at the code, I see various usages:
if (is_null($variable)) if (empty($variable)) if ($variable == NULL) if ($variable === NULL) if ($objectname->propertyname) if (array_key_exists($key, $arrayname)) if ($arrayname['key']) etc.
Can anyone point to or suggest a limited set of tests that we should use, and guidelines for when each should be applied? This would be a useful addition to the coding standards, http://drupal.org/node/318. Thanks!
-- Dondley Communications http://www.dondleycommunications.com Communicate or Die: American Labor Unions and the Internet http://www.communicateordie.com
I've reopened a bug report caused by this problem: http://drupal.org/node/20795 On 10/14/05, Steve Dondley <sdondley@gmail.com> wrote:
I was thinking, this problem also stems from the fact that sometimes fields have a NULL value in the database and sometimes are stored as empty strings. We will probably want to settle on a way of storing string in the database, too.
On 10/13/05, Nedjo Rogers <nedjo@gworks.ca> wrote:
Awhile back I posted a similar question, see below. We have a need for some standard guidelines on when to use which test for empty, null, or set variables, arrays, and object properties. _______
I find I'm sometimes not sure which types of test I should apply to test if a variable (or array key) is null or empty.
Looking at the code, I see various usages:
if (is_null($variable)) if (empty($variable)) if ($variable == NULL) if ($variable === NULL) if ($objectname->propertyname) if (array_key_exists($key, $arrayname)) if ($arrayname['key']) etc.
Can anyone point to or suggest a limited set of tests that we should use, and guidelines for when each should be applied? This would be a useful addition to the coding standards, http://drupal.org/node/318. Thanks!
-- Dondley Communications http://www.dondleycommunications.com
Communicate or Die: American Labor Unions and the Internet http://www.communicateordie.com
-- Dondley Communications http://www.dondleycommunications.com Communicate or Die: American Labor Unions and the Internet http://www.communicateordie.com
On Friday 14 October 2005 10:36 am, Steve Dondley wrote: Hello, I need your feedback on which of two solutions I should use to make Drupal E_STRICT compliant. http://drupal.org/node/28540
else if (isset($result)) {
to this:
else if (isset($result) && $result != '') {
This discussion is relevant to this: http://drupal.org/node/28540 (drupal base and E_NOTICE) An example of the error notice we get is when going to admin/settings: notice: Undefined index: #attributes in includes/form.inc on line 54. notice: Undefined index: class in includes/form.inc on line 54. the line is part of: function drupal_get_form ($form_id, &$form, $callback = NULL) which is called in this case from system.module from function system_view_general() Now I can solve this problem in by declaring the array this way: $form['#attributes']['class'] = array (); I can add this line 1- either within the function drupal_get_form in includes/form.inc 2- or within the function system_view_general in modules/system.module. Which way would be better/correct? Another similar example: notice: Undefined index: #built in includes/form.inc on line 180. should I change in the said line: if ($form['#built'] == TRUE) { to if (isset($form['#built'] AND $form['#built'] == TRUE) { or should I go back to where the function was called from, and declare the array there? By the way, looking at admin/settings, there are over 1480 such errors!!! most of them coming from form.inc. I'm working on updating the already proposed patches, but I need to know which way to go. thank you. -- http://www.wechange.org/ Because we and the world need to change. http://www.reuniting.info/ Intimate Relationships, peace and harmony in the couple. http://www.gnosis-usa.com/ Revolutionary Psychology, White Tantrism, Dream Yoga... http://www.masquilier.org/ Condorcet, Approval alternative, better voting methods.
On Fri, 14 Oct 2005 16:18:01 +0200, Anguo <linux-tw@masquilier.org> wrote:
On Friday 14 October 2005 10:36 am, Steve Dondley wrote: Hello,
I need your feedback on which of two solutions I should use to make Drupal E_STRICT compliant.
I do not want an E_STRICT Drupal. Buys us little. On the other hand, if you have too much time to burn, contact me, I can supply a horde of developers with tedious, boring but important issues. Regards NK
It buys us the ability to run under environments that are configured for E_STRICT, as many security-conscious people do. Drupal is the only code I write that isn't developed under E_ALL, because it's too far gone from it for me to fix it at the moment. On Friday 14 October 2005 03:50 pm, Karoly Negyesi wrote:
On Fri, 14 Oct 2005 16:18:01 +0200, Anguo <linux-tw@masquilier.org> wrote:
On Friday 14 October 2005 10:36 am, Steve Dondley wrote: Hello,
I need your feedback on which of two solutions I should use to make Drupal E_STRICT compliant.
I do not want an E_STRICT Drupal. Buys us little. On the other hand, if you have too much time to burn, contact me, I can supply a horde of developers with tedious, boring but important issues.
Regards
NK
-- 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 Saturday 15 October 2005 06:50 am, Larry Garfield wrote:
It buys us the ability to run under environments that are configured for E_STRICT, as many security-conscious people do.
Hello everyone, the patch doesn't solve everything, but it deals with a big chunk. http://drupal.org/node/28540#comment-49935 Could you review it, and tell me what I am doing wrong so that the patch can be committed soon? thanks.
An example of the error notice we get is when going to admin/settings:
notice: Undefined index: #attributes in includes/form.inc on line 54. notice: Undefined index: class in includes/form.inc on line 54.
the line is part of: function drupal_get_form ($form_id, &$form, $callback = NULL) which is called in this case from system.module from function system_view_general()
Now I can solve this problem in by declaring the array this way: $form['#attributes']['class'] = array (); I can add this line 1- either within the function drupal_get_form in includes/form.inc 2- or within the function system_view_general in modules/system.module.
Which way would be better/correct?
Better is to check for the existance of the form item before using it, not creating the item in every possible place. Since contributed code can result in similar errors.
Another similar example: notice: Undefined index: #built in includes/form.inc on line 180. should I change in the said line: if ($form['#built'] == TRUE) { to if (isset($form['#built']) AND $form['#built'] == TRUE) { or should I go back to where the function was called from, and declare the array there?
I don't know who have written "if ($form['#built'] == TRUE)", but AFAIK, this is the very same as "if ($form['#built'])" (since it is casted to a boolean either way. So shorter is to use "if (isset($form['#built']) && $form['#built'])" Goba
participants (7)
-
Anguo -
Gabor Hojtsy -
Karoly Negyesi -
Larry Garfield -
Nedjo Rogers -
Robert Douglass -
Steve Dondley