Re: PHP 7.1 - Address PHPSadness #28?

On Sep 16, 2015, at 14:09, Rowan Collins <rowan.collins@gmail.com> wrote: > I can certainly sympathise with wanting to run without any notices - they are generally hints > for writing better code. In the vast majority of cases, though, that means working out *why* the > variable is undefined, otherwise you might as well just use the @ operator to say "I don't > care about this notice”. Agree completely, and for that reason, we also ban @ except in those rare cases where PHP forces the issue (that’s for other discussions...). Indeed, this often comes up with short-lived issues that are resolved quickly. For example, a config file is missing a variable, so the user is told what it is, what kind of value to supply, and they goes off to fix it, or the user is warned and the app goes on with a default. But, all this happens without throwing errors and without having to detour through the global error handler. In other cases, the fix may not be so available, like with the template system that creates variables in the view’s scope; here, the goal is just to gracefully catch the condition and then define a default (and maybe complain to the author). > This still implies that there's a common problem to fix, which I just don't believe. > The number of places where the following all hold true must be vanishingly small: > > - the third-party code is dumping bare variables into your scope (globals, or a file-level > include are the only mechanisms I can think of) This seems to be pretty common with template systems, especially older ones. It’s also common with config files. > - they do so unreliably, in the sense that different variables will be set under different > circumstances More common than different variables in different circumstances is the missing variable. Consider the user who’s setting up an instance of an application that uses a config file with bare variables — very easy to forget something, or type the name wrong, or whatever. Or the programmer writing a plugin for a poorly documented application who wants to be sure the runtime environment of the plugin is what’s expected. Really, a lot of the same situations where one might need to check the existence of an array element or object property, except that the item to be checked happens to be a plain old variable. > - you/they have used null as a value which needs to be positively asserted, not taken as the > default state, so that isset() does not meet your needs Well, it’s not so important that null represents a non-default state, as that it represents any state. Often, null could just signal that a default should be used, but with an existence check, the programmer can be confidant that something/someone explicitly chose that option. In the case of a config, for example, you can be sure the user chose a particular value (which may be the default) versus having missed it altogether. There are cases where that knowledge is very important to safe programming. Consider that you’ve refactored a delete function to add support for non-permanent deletion. You’ve done this by adding an optional parameter: function DeleteFile($file, $permanentlyDelete = null) {} If $permanentlyDelete is null, the default action specified in a config is used. The problem is, when the param is null, there’s no way for the function to know for sure what the programmer of the caller intended. There are two possibilities: 1 The programmer intentionally passed null because the config should be used 2 The programmer forgot to consider that case after the refactor, so null is automatically being used Ultimately, the function just follows orders, of course, but doing it this way is dangerous because it’s all too easy to hit situation 2, which means files could be permanently deleted in error. As the programmer doing the refactor, it’s better to make the parameter mandatory, which forces a revisit to every caller to make the true/false/null decision explicit. In this case, the extra bit of intelligence comes from forcing the caller to pass something. If the param is optional, we have no idea. It’s the same thing with exists() on a variable: it gives us extra intelligence about *why* a variable is null, whether it’s because of an actual decision somewhere or because of an error or oversight. And it provides that intelligence without fussing with PHP errors. > - you have no way of presetting the variable to some other terminal value to detect when it is > explicitly set If the variables are injected before your code runs, it’s too late. At that point, if you try to set a magic sentinel value, you’re just overwriting the real value if there is one. It would work, however, if you can pre-initialize before the injection: $foo = 'hope this gets overwritten'; require('/some/config.php'); if ($foo === 'hope this gets overwritten') { //uh-oh } I’m not really a fan of this pattern, though. If there’s going to be a magic value to indicate that no better value was set, null is the ideal choice: $foo = null; require('/some/config.php'); if ($foo === null) { //uh-oh } Here, we’ve used null exactly as intended. Unfortunately, this breaks if null already has meaning, even if it's just "go with a default”. And, of course, it also throws the annoying undefined error that we’re trying to avoid :-). This is where exists() comes in and cures all: require('/some/config.php'); if (!exists($foo)) { //uh-oh } It’s cleaner and shorter, but it also lets us again use null as intended, rather than coming up with some whacky magic value that by necessity is completely unrelated to the variable’s purpose yet is hopefully understandable in meaning to future programmers passing through (not *too* hard with a string, as above, but much tricker with non-strings). >> In a dynamic >> language like PHP, I really don’t understand how folks can *not* see >> that as an oversight. > > I've not found evidence that any other language treats variables in the proposed way, so > it really doesn't feel like an "oversight" to me, but a request for a new and quite > unusual feature. In JavaScript, there’s this: if (typeof never_heard_of_me === 'undefined') { //uh-oh } albeit with a couple of minor caveats. > But yes, from a completeness point of view, it does make some sense. My fear is that the people > who don't understand why isset() does what it does will think it's the answer to some > imaginary problem, and end up more confused than ever when their code breaks. Haven’t we already arrived here with isset() confusion? ;-) Seriously, I think updating the docs on the isset() page would help mitigate this. From this: "Determine if a variable is set and is not NULL. "If a variable has been unset with unset(), it will no longer be set. isset() will return FALSE if testing a variable that has been set to NULL. Also note that a null character ("\0") is not equivalent to the PHP NULL constant." to something like this: "Determine if a variable exists in the current scope and is not NULL. To test only whether a variable exists, use exists(). "If unset() is called on a variable, it will no longer exist. isset() will return FALSE if testing a variable that has been set to NULL. Also note that a null character ("\0") is not equivalent to the PHP NULL constant." Obviously, other related pages could be updated similarly. > If the function had been proposed as a debugging aid, maybe filed under Reflection or > debug_variable_is_initialised, I might have more sympathy, but that's not how it's been > presented. Again, I don’t see this as something that directly controls business logic. I see it as something that supports defensive programming, improved error handling, and improved user experience. Regardless, as we all know, every language feature can be abused, but that doesn’t automatically mean we shouldn’t have those features. Even something as widely derided as goto has its niche, after all (albeit, a very, very small niche). In this case, not only is the feature useful, but it nicely complements the array of related functionality to provide a more complete feature set. -Bob

Attachment: [application/pgp-signature] Message signed with OpenPGP using GPGMail signature.asc