Edit report at http://bugs.php.net/bug.php?id=52874&edit=1

 ID:                 52874
 Comment by:         cataphr...@php.net
 Reported by:        cataphr...@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] cataphr...@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] cataphr...@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

Reply via email to