On 9 Sep 2015, at 19:34, Rowan Collins <rowan.coll...@gmail.com> wrote:
> Craig Francis wrote on 09/09/2015 16:54:
>> I don't think it is an odd thing to suppress a warning for a plain variable.
>>
>> For example, on a blog you may have a list of articles, which is used by
>> both the admin and users, but the (MVC) controller might set an $add_url, so
>> a link to add a new article can be displayed...
>>
>> Controller:
>>
>> if (IS_ADMIN) {
>> $this->set('add_url', '/admin/articles/add/');
>> }
>>
>> View:
>>
>> <?php if (exists($add_url)) { ?>
>> <p><a href="<?= html($add_url) ?>">Add article</a>
>> <?php } ?>
>
> I can see that this makes nice terse View code, and is a reasonable use of
> existing isset() - though I can't see it needs to be anything other than
> isset(), because if $add_url was null, you wouldn't want to echo anything.
>
> Arguably, it would be better handled with the vars placed into an array
> rather than materialising as PHP variables. The helper functions defined by
> the framework (here assumed to be called exists() and html()) could then
> extract items from that array, making it just as succinct:
>
> <?php if (exists('add_url')) { ?>
> <p><a href="<?= html('add_url') ?>">Add article</a>
> <?php } ?>
>
>
> Boom, no more worries about undefined variables.
It depends, the exists() was simply to show the proposed function in place, and
html() was more to show a cut down version of htmlentitites.
Most frameworks I've used typically have plain variables in the View
(templating engines are a bit different)... but I have used a $v[] array on my
own project before (many years ago), but it got quite ugly... especially when
it's often passing multi-dimentional arrays (or objects) into the View.
So yes, in the example above, an isset() would work, but I was trying to show
why I don't think it's odd to "suppress a warning for a plain variable" when it
does not exist.
> I'm pretty sure this is how templating languages like Smarty and Twig work -
> "{$foo}" in Smarty compiles to something like "echo $this->vars['foo'];"
>
> Presumably under the hood, the set() method in your example has to instead do
> some magic with variable variables - $$foo syntax - which is like the ugly
> cousin of an associative array. Either that, or eval(), which as everyone
> knows is evil()! :P
Actually, I think they do use an array to store the values, then do an
extract() to make them local variables in the View (keeping the variable names
shorter and easier to read).
The code I'm looking at today uses a function to include the View, such as:
function script_run() {
if (func_num_args() > 1) {
extract(func_get_arg(1));
}
require(func_get_arg(0));
}
script_run($view_path, $variables);
>> But, the thing that wins it over for me is that isset() seems to be
>> (mis)used by a lot of developers as a function to check if the variable
>> exists (not that the variable has a value).
>
> Do you mean that it is misused with arrays? Because with plain variables it
> does exactly what it needs to do.
>
> Or do you mean that developers are using unset() to represent a state
> distinct from null? Because if so, that is their mistake, not the subsequent
> use of isset(). I have yet to see a single use case where distinguishing
> between these two states is necessary or sensible.
Yes, it is mis-used with arrays, but I would still argue that a single exists()
function would work better for all of these examples when testing if they exist.
And I would hope that no-one was using an unset() as something distinctly
different from a NULL value.
>> Actually, in a few years time, if developers did their NULL checks with "===
>> NULL", then isset(), is_null(), array_key_exists(), and empty() could all be
>> deprecated :-)
>
> - is_null() makes sense because it's a type-checking function: it goes with
> is_bool(), is_int(), etc.
> - empty() is a bit odd because it's basically an inverted boolean cast, but
> does make quite readable code.
> - You've just demonstrated yourself a situation where isset() can't be
> replaced with === null or is_null() because you want to suppress the Notice.
> - I am fundamentally opposed to a function which distinguishes between
> unset($foo) and $foo=null for reasons I have stated several times.
>
> So the only thing left is replacing array_key_exists with some nicer syntax
> so you can write some_keyword($foo['bar']) rather than
> array_key_exists('bar', $foo)
I agree that we should not have a function that distinguishes between
unset($foo) and $foo=null.
But I believe the isset() function is often used just to see if the variable
exists (i.e. avoiding the undefined variable notice), and having the extra "and
is not NULL" logic does not help in this situation.
It's just the "and is not NULL" logic causing problems when the developer
should be using array_key_exists.
So if we look at it from the point of view of "lets kill isset" (just humour me
for a second)...
The proposed exists() function should be the go to function, and used to avoid
undefined variable notices (e.g. the View variables).
Then to check if the variable is NULL, just do a "=== NULL", which is much
better because it will complain if the variable is undefined (or you get your
wish, and isset() could be updated to raise a notice for undefined variables).
Anyway, I'm off on holiday tomorrow, so I doubt I will be able to follow up for
a couple of weeks.
Craig
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
