ZF2-381: Use "assert" when it's possible
Description
Everywhere in the code, developers checks parameters type ou return values with "if" + "instanceof/is_array/is_string/etc." + "throw exception". It's not a good idea for performance because, this check will stay when configuration/implementation will be stabilized.
Do you know PHP "assertion" ? Use it please :)
Comments
Posted by Matthew Weier O'Phinney (matthew) on 2012-06-28T21:18:28.000+0000
There are a variety of reasons to use conditionals + throw. Using assert() does not necessarily change that -- we still have to check if the condition is false, and notify the caller (typically via an exception).
If you can demonstrate tangible benefits -- specifically, a pull request, with benchmarks -- we can consider this change. That said, it's a broad change, and not one likely to get support unless you or somebody else invested in such a change is willing to drive it through contributions.
Posted by Matthew Weier O'Phinney (matthew) on 2012-06-28T21:20:52.000+0000
Additionally, please read the following:
That last link, from the PHP manual itself, provides some interesting feedback contrary to your request:
{quote} Assertions should be used as a debugging feature only. You may use them for sanity-checks that test for conditions that should always be TRUE and that indicate some programming errors if not or to check for the presence of certain features like extension functions or certain system limits and features.
Assertions should not be used for normal runtime operations like input parameter checks. As a rule of thumb your code should always be able to work correctly if assertion checking is not activated. {quote}
Posted by Matthew Weier O'Phinney (matthew) on 2012-06-28T21:22:01.000+0000
Closing, as requires somebody to drive, and due to evidence indicating request may not have the intended effect.
Posted by Frederic Bouchery (bouchery) on 2012-06-29T07:39:44.000+0000
I'm fighting every single day to make it clear to developers the difference between user data checking and developer design checking. The PHP manuel is a little bit wrong : "assert" is not a "debugging" feature, but a code design check, like type hint. Example : <?php function MyFuntion(MyClass $object, array $array, $string, $int) { assert('is_string($string) and is_int($int)'); ///... } ?>
If you are using "type hint", you can understand "assert" : when code design is stabilized, on production server, assert are deactivated, and no more check are performed => performance.
Here a sample from ZF2 module manager in loadModule method :
<?php // .... $module = $result->last(); if (!is_object($module)) { throw new Exception\RuntimeException(sprintf( 'Module (%s) could not be initialized.', $moduleName )); } // .... ?>When development and configuration are stabilized, why continue to test if "$module" is an object because, it will be true each time ? In this case, "assert" is a kind of "type hint" for method result. I think, each time you use "RuntimeException", it can be replace with an assert.
I can drive these changes if you want, but it is necessary for other developers to understand "design by contract" and "assertion" to stop using "RuntimeException".
Frederic - Developer since 1983, PHP developer since 1998, architect and lead developer for ages :)
Posted by Frederic Bouchery (bouchery) on 2012-07-04T11:52:17.000+0000
My apologies, I performed some benchmark, and I noticed "assert(instanceof)" is 20 times slower than "if(!instanceof)" with assertions disabled (and 50 times when enabled).
My whole world is falling apart ;)
Posted by Matthew Weier O'Phinney (matthew) on 2012-07-05T13:29:35.000+0000
Frederic -- that was the conclusion in the first link I provided as well -- that performance of assert() is very bad regardless of whether or not it is enabled (the author there noticed only a 25% improvement between enabling and disabling, and these numbers were still many times worse than not using assert() at all).
I suspect this may be part of the reasoning behind the note in the PHP manual recommending them for development and debugging only.