protected function setParams( array $params = array() ) {
assert(call_user_func(function()use($params){
// Name is required.
if (!isset($params['name']) || !is_string($params['name']) || $params['name'] == '') {
return false;
}
// These must be boolean if set.
foreach (array ('allowNull', 'unique', 'required', 'flag') as $key) {
if (isset($params[$key]) && !is_bool($params[$key])) {
return false;
}
}
if (
(isset($params['minimum']) && !is_numeric($params['minimum'])) ||
(isset($params['maximum']) && !is_numeric($params['maximum'])) ||
(isset($params['values']) && !is_array($params['values']))
){
return false;
}
return true;
}));
foreach ( array ('name', 'default', 'minimum', 'maximum', 'allowNull',
'unique', 'required', 'flag', 'values', 'encrypt') as $key ) {
$this->$key = isset($params[$key]) ? $params[$key] : $this->$key;
}
}
Assert is used here because in normal operation the parameters passed to this constructor should be valid, and everything the closure does shouldn’t need to be checked.
That said, my understanding is that when is assertion is turned off none of the code within the assertion is evaluated at all effectively making the assertion code comment text. This is correct yes?
I’m also wondering if I’m just trying to be too clever here.
That said, my understanding is that when is assertion is turned off none of the code within the assertion is evaluated at all effectively making the assertion code comment text. This is correct yes?
It looks that way. ASSERT is a macro likely having origins in C and /or other pre-processor langauges.
It’s an archaic practice, IMO. I used to do this all the time when I first came to PHP from no other than C/C++. It works well in pre-compiled setups because ASSERT is only evaluated in DEBUG mode and in RELEASE the code is completely removed from the output.
In PHP this is not the case as your code is still there and still requires parsing (despite being cached if APC and never being executed) and maintenance. It’s an example of a cross cutting concern, IMO.
It seems you are trying to verify certain parameters are set, because of the use of array arguments, so the obvious alternative is to use standard function arguments and type hinting if illegal ‘type’ can cause an issue?
Your tests should catch this kind of error and in addition - keeps the code externalized and non-conflicting with real purpose of the method.
Since expressions get parsed from the inside out, it will run everything inside the closure still, but with assertions disabled, it won’t cause a fuss when if it’s false.
Historically assertions are used to provide design by contract. Which essentially includes tests within the method (mostly using pre/postconditions). While it offers very nice self-documenting code I find it creates too much clutter and would rather apply separation of concerns and do testing independently.
In DBC the assertions are turned off in a production environment for performance and to stop the end user seeing errors.
Keep in mind you’ll probably also want an exception for the production environment just in case something happens that prevents the method being able to continue.
If the data passed to the method is not usable, it should throw an exception. Assertions sort of make sense to me if they are left in all the time, but the fact that it’s checked once (randomly, based on environment), and then turned off is a problem to me. While the database may be working, it could still return possibly corrupt or incomplete data. The fact that there are assertions proves the possibility of an error condition. An error condition that is not handled.
If it’s possible for the error conditions to happen in the first place, something should be left there permanently to handle them; and that’s an exception - not an assertion.
That is NOT this object’s job, ok? Quit trying to poke holes where non exist, especially when you don’t know where this fits in the rest of the system. Criticizing this function for not having input checking code is as blatantly stupid as criticizing the Database class for being unable to parse HTML template files.
Here, let me spell this out for you two in big letters…
protected function
There’s a reason for that. I’m not an idiot. Quit trying to paint me as one.
Only one function calls this - the constructor, possibly a child class’s version of the same.
Now, as I said before I took the assertion out and will let the factory class deal with the validity of the parameters.
Anyway, my point still stands, you don’t know what any subclasses will pass to that function, it may also be used several times within the class in a subclass. All I’m suggesting is that by moving input checking to the function itself you future-proof yourself against repeated code.
I’m sure if I did that and posted it someone else ( probably you )would chime in that such is a cross cutting concern and such logic should be farmed off to the factory method for the object (which is coincidently what I’m doing).
What I’m snapping at is answers not germane to the original question - the idea of using full closures in an assert call and whether such closures have to be compiled when assert is turned off. I didn’t invite the subsequent question of whether or not the class should review the parameters it has been given, I certainly didn’t invite the additional badgering following the statement that I was going to move that logic to the handling factory class where it belongs. When both the factory and the object itself review params that’s busybox coding and duplication of effort. One or the other should make the check.
Nevertheless, your comment seems to be a display of extreme annoyance and frustration. It’s common for people (more specifically programmers) to go into things that aren’t the main purpose of your topic, but you have to understand they’re trying to help you. Snapping like that seems impolite, but truth be told: I don’t know the full history.
I’m not even sure it is off topic. In the OP you provide a method of checking the method is able to work with its given parameters. The rest was just a discussion of how else this can be achieved…