Re: svn: /php/php-src/ branches/PHP_5_4/Zend/zend_compile.c trunk/Zend/zend_compile.c

Hi: On 18 Nov 2011, at 15:16, Felipe Pena wrote: > Hi, > > 2011/11/18 Stefan Marr <php@stefan-marr.de>: >> Hi Felipe: >> >> On 18 Nov 2011, at 14:56, Felipe Pena wrote: >> >>> felipe Fri, 18 Nov 2011 13:56:41 +0000 >>> >>> Revision: >>> http://svn.php.net/viewvc?view=revision&revision=319487 >>> >>> - else if (!strncmp(mname, ZEND_DESTRUCTOR_FUNC_NAME, mname_len)) { >>> ce->destructor = fe; fe->common.fn_flags |= ZEND_ACC_DTOR; } >>> - else if (!strncmp(mname, ZEND_GET_FUNC_NAME, mname_len)) ce->__get >>> = fe; >>> - else if (!strncmp(mname, ZEND_SET_FUNC_NAME, mname_len)) ce->__set >>> = fe; >>> - else if (!strncmp(mname, ZEND_CALL_FUNC_NAME, mname_len)) ce->__call >>> = fe; >>> - else if (!strncmp(mname, ZEND_UNSET_FUNC_NAME, mname_len)) ce->__unset >>> = fe; >>> - else if (!strncmp(mname, ZEND_ISSET_FUNC_NAME, mname_len)) ce->__isset >>> = fe; >>> - else if (!strncmp(mname, ZEND_CALLSTATIC_FUNC_NAME, mname_len)) >>> ce->__callstatic= fe; >>> - else if (!strncmp(mname, ZEND_TOSTRING_FUNC_NAME, mname_len)) >>> ce->__tostring = fe; >>> - else if (ce->name_length + 1 == mname_len) { >>> + } else if (!strncmp(mname, ZEND_DESTRUCTOR_FUNC_NAME, mname_len)) { >>> + ce->destructor = fe; fe->common.fn_flags |= ZEND_ACC_DTOR; >>> + } else if (!strncmp(mname, ZEND_GET_FUNC_NAME, mname_len)) { >>> + ce->__get = fe; >>> + } else if (!strncmp(mname, ZEND_SET_FUNC_NAME, mname_len)) { >>> + ce->__set = fe; >>> + } else if (!strncmp(mname, ZEND_CALL_FUNC_NAME, mname_len)) { >>> + ce->__call = fe; >>> + } else if (!strncmp(mname, ZEND_UNSET_FUNC_NAME, mname_len)) { >>> + ce->__unset = fe; >>> + } else if (!strncmp(mname, ZEND_ISSET_FUNC_NAME, mname_len)) { >>> + ce->__isset = fe; >>> + } else if (!strncmp(mname, ZEND_CALLSTATIC_FUNC_NAME, mname_len)) { >>> + ce->__callstatic = fe; >>> + } else if (!strncmp(mname, ZEND_TOSTRING_FUNC_NAME, mname_len)) { >>> + ce->__tostring = fe; >>> + } else if (ce->name_length + 1 == mname_len) { >> >> How is that change an improvement for readability? >> The previous formatting was an implicit tabled arrangement, which makes it very easy to see >> what the commonalities are and where the code differs. >> Breaking that up into multiple lines, does IMHO not improve readability or clarity. If you >> just missed the braces, well, they could have been added inline, no? >> And I do not really buy that 'CS' (that is coding style?) applies to such a >> special piece of code. >> >> Would it be a problem to revert that change, or at least 'table' it again? > > CS = Coding standards > > Well, our codind standards in the CODING_STANDARDS file states: > > > -------8<----------------------------------------------------------------------------------------------------------- > > 3. Be generous with whitespace and braces. Keep one empty line between the > variable declaration section and the statements in a block, as well as > between logical statement groups in a block. Maintain at least one empty > line between two functions, preferably two. Always prefer:: > > if (foo) { > bar; > } > > to: > > if(foo)bar; > > > -------8<----------------------------------------------------------------------------------------------------------- > > So I just did it in conforming whole rest of the code. It's not a > question of readability, but the following an adopted standards. Then at least follow what 3. states as the first (and deducted from that) most important statement: "Be generous with whitespace" Please, make it look like a table. That is not against the coding standards. (Which IMHO should leave room for exceptions when it comes to readability) Be generous with whitespace! Thanks. Stefan -- Stefan Marr Software Languages Lab Vrije Universiteit Brussel Pleinlaan 2 / B-1050 Brussels / Belgium http://soft.vub.ac.be/~smarr Phone: +32 2 629 2974 Fax: +32 2 629 3525