Req #52874 [Com]: Refactor big if condition w/ collateral effects and nested ternary statements
| From: | cataphract@php.net | Date: | Sat, 18 Sep 2010 01:31:53 +0000 |
| Subject: | Req #52874 [Com]: Refactor big if condition w/ collateral effects and nested ternary statements | ||
| References: | 1 | Groups: | php.bugs |
| Request: | Send a blank email to php-bugs+get-154470@lists.php.net to get a copy of this message | ||
Edit report at http://bugs.php.net/bug.php?id=52874&edit=1
ID: 52874
Comment by: cataphract@php.net
Reported by: cataphract@php.net
Summary: Refactor big if condition w/ collateral effects and
nested ternary statements
Status: Open
Type: Feature/Change Request
Package: Scripting Engine problem
Operating System: Irrelevant
PHP Version: trunk-SVN-2010-09-18 (SVN)
Block user comment: N
New Comment:
It's not just non-obvious. It also makes debugging harder. If I segfault
on the if condition, my debugger will show it as occurring in the last
line of the if.
Previous Comments:
------------------------------------------------------------------------
[2010-09-18 03:27:51] cataphract@php.net
The following patch has been added/updated:
Patch Name: refact_if
Revision: 1284773271
URL:
http://bugs.php.net/patch-display.php?bug=52874&patch=refact_if&revision=1284773271
------------------------------------------------------------------------
[2010-09-18 03:27:28] cataphract@php.net
Description:
------------
Two if conditions (one in zend_std_read_property and another in
zend_std_write_property) are very confusing and should be refactored.
Here's one:
493 if (EXPECTED(property_info != NULL) &&
494 ((EXPECTED((property_info->flags & ZEND_ACC_STATIC) == 0)
&&
495 property_info->offset >= 0) ?
496 (zobj->properties ?
497 ((variable_ptr =
(zval**)zobj->properties_table[property_info->offset]) != NULL) :
498 (*(variable_ptr =
&zobj->properties_table[property_info->offset]) != NULL)) :
499 (EXPECTED(zobj->properties != NULL) &&
500 EXPECTED(zend_hash_quick_find(zobj->properties,
property_info->name, property_info->name_length+1, property_info->h,
(void **) &variable_ptr) == SUCCESS)))) {
501 /* if we already have this value there, we don't actually need
to do anything */
This is very non-obvious.
I've refactored the conditions, which I tested with no regressions in
the tests.
------------------------------------------------------------------------
--
Edit this bug report at http://bugs.php.net/bug.php?id=52874&edit=1