Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:88179
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain craigfrancis.co.uk designates 209.85.212.170 as permitted sender)
Content-Type: text/plain; charset=windows-1252
Mime-Version: 1.0 (Mac OS X Mail 7.3 \(1878.6\))
In-Reply-To: <55F07BA4.2000204@gmail.com>
Date: Mon, 14 Sep 2015 12:46:43 +0100
Cc: internals@lists.php.net
Content-Transfer-Encoding: quoted-printable
Message-ID: <E8634CF1-C2B4-4C5A-8DDC-531D9599774B@craigfrancis.co.uk>
References: <CAKws9z27kVJ87Ur5Qo+ayYHPeeO=aYKpRHUV4TFBXX2iMcgO2A@mail.gmail.com> <55DD4269.4090402@gmail.com> <6348DFA7-04BD-41BB-A500-17A8A531B56C@craigfrancis.co.uk> <55DDA4C9.9040603@gmail.com> <3C69BF4B-52E6-4D04-8601-8D23DFCC538E@craigfrancis.co.uk> <55DDD60F.5090509@gmail.com> <8C74463E-DBA2-4015-8159-0B44D973387F@craigfrancis.co.uk> <55DE0907.6040904@gmail.com> <1F615BCD-1B9B-4C51-A210-869F1AA1F6E3@craigfrancis.co.uk> <55E5EBBF.6020803@gmail.com> <0BA3A129-D356-4781-B6DE-E2B5A7924AE2@craigfrancis.co.uk> <55E6EC36.6090301@gmail.com> <E1C02A1A-EB7C-40D9-8760-13627CC7BF85@craigfrancis.co.uk> <9AF329EC-99A5-412D-A52B-432627A5520F@gmail.com> <6F4D91EE-B56E-4B83-B1AF-598C3F6897FC@craigfrancis.co.uk> <55F07BA4.2000204@gmail.com>
To: Rowan Collins <rowan.collins@gmail.com>
Subject: Re: [PHP-DEV] PHP 7.1 - Address PHPSadness #28?
From: craig@craigfrancis.co.uk (Craig Francis)


On 9 Sep 2015, at 19:34, Rowan Collins <rowan.collins@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.
>>=20
>> 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...
>>=20
>> Controller:
>>=20
>>     if (IS_ADMIN) {
>> 	$this->set('add_url', '/admin/articles/add/');
>>     }
>>=20
>> View:
>>=20
>>     <?php if (exists($add_url)) { ?>
>> 	<p><a href=3D"<?=3D html($add_url) ?>">Add article</a>
>>     <?php } ?>
>=20
> 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.
>=20
> 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:
>=20
>    <?php if (exists('add_url')) { ?>
> 	<p><a href=3D"<?=3D html('add_url') ?>">Add article</a>
>    <?php } ?>
>=20
>=20
> 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'];"
>=20
> 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).
>=20
> Do you mean that it is misused with arrays? Because with plain =
variables it does exactly what it needs to do.
>=20
> 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 "=3D=3D=3D NULL", then isset(), is_null(), array_key_exists(), and =
empty() could all be deprecated :-)
>=20
> - 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 =3D=3D=3D 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=3Dnull for reasons I have stated several times.
>=20
> 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=3Dnull.

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 "=3D=3D=3D 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=