Sorry for the 11th hour chime-in, but I wanted to add some context
about this proposed change, and how it would likely affect our project
and many others. I know the prospect of opt-in was already addressed,
but I wanted to talk about what our experience would be here. (We make
Snipe-IT, which is open-source but also a paid-for-hosting or
pay-for-support project).
Many modern PHP projects (like ours) have 10's or 100's of
dependencies, often installed via Composer. With this change, many of
those will start to throw deprecation warnings - eventually failing in
PHP v9. Not every package is equally modern, or equally
well-maintained. I suspect there are plenty of newer packages that
will "Just Work™" and that's great! But there are probably many older
packages, or more poorly-maintained ones, that will not.
Packagist has around 3 million packages. Figure half are "not modern"
(needing updates to be compatible with the proposed change) - that'd
be 1.5 million packages. (And that's probably pretty conservative).
Figure it takes an hour, each, to fix them all (again, that's
conservative). That's 1.5 million hours, which is 171 developer-years.
That's effectively 2 or 3 developer lifetimes (probably more, if we
try to include work-life balance and whatnot :P )
And what are 9 out of 10 maintainers going to do? They'll just plaster
#[AllowDynamicProperties] in front of every class, and be done with
it. Voila! Compatibility. So the bulk of those 171 developer-years
will be wasted.
I think opt-in would be a better approach - per-class, certainly
(#[FixedProperties] or #[StaticProperties] maybe?), but maybe per-file
as well, or maybe even in php.ini if we want to go per-project.
Different language front-ends to the php back-end engine could
certainly by default jam the attribute in on every class; that'd be
fine. But for PHP itself, one of its massive selling points has been
that it tries very hard not to break compatibility - I have written
code for PHP v4 that I have been able to drag up to PHP v7.x without
too much work. I'd hate to lose that, or make it much harder -
especially for dependencies that I don't have direct control over.
By no means am I suggesting that PHP is a 'dead language' - but let's
not cram ourselves into a Python2/Python3 compatibility hole where we
make upgrades into a nightmare for people. Adding types for variables
and parameters has been optional and is gradually becoming the
standard thing to do; I'm sure this would end up the same.
Anyways, hoping to just add some more context for those who are voting
on the proposal. Feel free to ask any questions you'd like and I'll be
happy to answer as best I can.
Hi Brady,
This is a little bit overly dramatic. This isn't such a huge change that
would affect 50% of existing projects. It's likely to affect a small number
of projects in a very limited way.
It's also not true that developers will slap #[AllowDynamicProperties] on
every class. That would imply that every class in their project is using
dynamic properties. That would be absurd. If it's truly the case that every
class in 50% of the projects uses dynamic properties then we should not
consider this RFC at all. The whole premise of it is that it's highly
unlikely that many people are using dynamic properties on purpose. It's
true that many old unmaintained projects might use it as a feature, but the
keyword is "unmaintained". I would not trust dependencies that haven't been
kept up to date for a few years.
PHP has had many breaking changes over the years. PHP 8.1 introduced a much
more annoying deprecation: Deprecate passing null to non-nullable arguments
of internal functions. This one I would have no trouble believing that it
impacts 50% or even the majority of PHP projects. Dynamic properties
compared to this should be a piece of cake.
I doubt that having to add a single attribute to a selected few classes
would be such a major issue that projects would decide to stay on PHP 8.1
rather than upgrade. It's not comparable to Python 2/3 situation IMHO.
I would be interested to know some numbers from Snipe-IT. Have you done a
preliminary analysis of your codebase of how many classes are actually
using dynamic properties? From a brief look, I can see that usually most
properties are declared.
Regards,
Kamil
This is a little bit overly dramatic. This isn't such a huge change that would affect 50% of existing projects. It's likely to affect a small number of projects in a very limited way.
It's also not true that developers will slap #[AllowDynamicProperties] on every class. That would imply that every class in their project is using dynamic properties. That would be absurd. If it's truly the case that every class in 50% of the projects uses dynamic properties then we should not consider this RFC at all. The whole premise of it is that it's highly unlikely that many people are using dynamic properties on purpose. It's true that many old unmaintained projects might use it as a feature, but the keyword is "unmaintained". I would not trust dependencies that haven't been kept up to date for a few years.I doubt that having to add a single attribute to a selected few classes would be such a major issue that projects would decide to stay on PHP 8.1 rather than upgrade. It's not comparable to Python 2/3 situation IMHO.
I would be interested to know some numbers from Snipe-IT. Have you done a preliminary analysis of your codebase of how many classes are actually using dynamic properties? From a brief look, I can see that usually most properties are declared.
Sorry, I didn't mean to sound dramatic, just trying to share my concerns.
I suspect that you're very right, and very very few projects use
dynamic properties on purpose (in our own code, we probably don't ever
mean to, except maybe for our Custom Fields feature, but we could do
the attribute for that one or so class(es)), but that's not my point.
I suspect many of our dependencies might be using it (by accident,
to your point!). But I can't control every package in my
composer.json, which is where I'm worried. I can't force those
developers to do what I want, and I don't want to be stuck in a
'version sandwich' because I'm waiting for one of them to change
something. And maybe when they do try and change something, they
might also change the interface (a violation of semver, but, it
happens in the wild, more often than we would like). We have to
support old and new versions of php and that can make our Composer
situation a real nightmare (I just dragged us forward to be able to
work in PHPv8, and it was far harder than it needed to be).
I'm not worried about our code at all - we write it! We can change
it (and we'd be good citizens, I promise, we would!) And that
attribute is ignored on versions of PHP that don't support it; which'd
be fine for us. That's a fair ask, and we'd do it.
What I'm scared about is about our 42 dependencies in composer.json,
and the ~400k bytes of dependencies described by our composer.lock.
And I'm worried about other projects that have even more dependencies,
or might be stuck on a framework or something that's no longer being
updated, but they can't migrate off of for whatever reason. I wouldn't
wish that on anyone.
Thank you for considering, and for your extremely well-written response,
-B.
On Fri, 26 Nov 2021 at 00:55, Brady Wetherington via internals
internals@lists.php.net wrote:
That's 1.5 million hours, which is 171 developer-years.
If we're going to imagine numbers; there are 6 million PHP developers
in the world*. If on average they each lose just 1 hour per year by
making typos and accidentally creating a properties dynamically,
that's 6 million hours, or 684.93 years!
So the value delivered by this change would be 4 times the cost just
in the first year. And then every year after that it's pure benefit.
What I'm scared about is about our 42 dependencies in composer.json,
How about sponsoring each of your dependencies some money, to
encourage them to check if their code is compatible after this change,
and fix it if it isn't.
If a dependency is used by even just 1000 companies, and each of those
companies chips in $50, then $50,000 will fund many months of work on
that dependency.
probably more, if we try to include work-life balance and whatnot :P
Most open source is done by people in their free time. Because
companies keep refusing to fund open source.
might be stuck on a framework or something that's no longer being updated,
Having companies sponsor open source projects makes it less likely
they will be abandoned.
cheers
Dan
Ack
That's 1.5 million hours, which is 171 developer-years.
If we're going to imagine numbers; there are 6 million PHP developers
in the world*. If on average they each lose just 1 hour per year by
making typos and accidentally creating a properties dynamically,
that's 6 million hours, or 684.93 years!So the value delivered by this change would be 4 times the cost just
in the first year. And then every year after that it's pure benefit.
And as many of those who wish to, can opt-in to the attribute/feature
can save themselves that time (and probably should!). We probably will
for our code, tbh! But forcing this issue is going to be massively
destructive to the ecosystem. Not everything is brand-spanking new.
Some things just work well and don't get updated that often.
What I'm scared about is about our 42 dependencies in composer.json,
How about sponsoring each of your dependencies some money, to
encourage them to check if their code is compatible after this change,
and fix it if it isn't.
You are trying to solve a technological problem with an economic
solution. And one that requires that the entire ocean be boiled before
it is solved. We already do give money to several places, and would
like to give more. And we will. But that's not going to fix this,
unless everyone does. And not everyone will. We happen to make money;
not a lot of other projects do.
We ourselves are open source. We don't get more than a handful of
donations per year. This doesn't seem to be the right way to solve
this problem.
If a dependency is used by even just 1000 companies, and each of those
companies chips in $50, then $50,000 will fund many months of work on
that dependency.Most open source is done by people in their free time. Because
companies keep refusing to fund open source.might be stuck on a framework or something that's no longer being updated,
Having companies sponsor open source projects makes it less likely
they will be abandoned.
Maybe, but not always. Sometimes people just lose interest. But again,
you're proposing to potentially break a big chunk of packages in hopes
that an economic model that has not successfully worked anywhere else
is going to work here. That seems cavalier.
Right now, to get a collection of packages that work between php 7.4
and 8.0, we've had to spend tremendous amounts of work just to get
there - and we had to pull 7.3 support in the process. I didn't want
to do that; I want our software to be available to as many people as
possible - not everyone has the freedom to install their own version
of PHP - some are on crappy shared hosting or have to deal with some
creaky IT department. Every single backwards-incompatible change
shrinks our version-sandwich down further, especially in the context
of packages. Again, I can add the new attributes to our own software
today, and that means we're OK - but I can't speak to every single
package we're dependent on or they are dependent on.
-B.
On Fri, Nov 26, 2021 at 12:16 AM Brady Wetherington via internals <
internals@lists.php.net> wrote:
That's 1.5 million hours, which is 171 developer-years.
If we're going to imagine numbers; there are 6 million PHP developers
in the world*. If on average they each lose just 1 hour per year by
making typos and accidentally creating a properties dynamically,
that's 6 million hours, or 684.93 years!So the value delivered by this change would be 4 times the cost just
in the first year. And then every year after that it's pure benefit.
The point of this counter-argument which may have gotten lost in the snark
is that nobody has done the legwork to know definitively what the impact of
this will be. You're both making up numbers, and this is part of the
reason the deprecation warnings exist, so that concerned product developers
with a stake in making sure these sorts of things go smoothly have time to
test such changes out on your own codebases. It's been merged to master,
so you could stand up a build now and point to the many deprecation
warnings you're expecting. I'm not saying send PRs to fix them all, just
show real impact rather than theoretical guessing. If it's as bad as you
fear, maybe we run another RFC to remove the change before it ever sees the
light of day and you become a super hero. Call you Uber Brady. Or maybe
not a single deprecation warning shows up. Or just a couple that are easily
fixed. Let's make PHP better, together.
-Sara
…It's been merged to master, so you could stand up a build now and point
to the many deprecation warnings you're expecting. I'm not saying send PRs
to fix them all, just show real impact rather than theoretical guessing. …
That’s a totally fair ask, and I’ll try and get that done and report back.
I think I’m probably going to run into some Laravel problems here, as they
only just came up with a version that will run under php 8.1. I’ll get as
far as I can and let you know - even if I determine that my suspicions were
completely incorrect :)
…It's been merged to master, so you could stand up a build now and point to the many deprecation warnings you're expecting. I'm not saying send PRs to fix them all, just show real impact rather than theoretical guessing. …
That’s a totally fair ask, and I’ll try and get that done and report back. I think I’m probably going to run into some Laravel problems here, as they only just came up with a version that will run under php 8.1. I’ll get as far as I can and let you know - even if I determine that my suspicions were completely incorrect :)
Okay - good news/bad news/good news -
Good News: I managed to snag an 8.2 build and ran Snipe-IT on it, and
it worked completely fine. I cheated a little bit - I didn't try to
make a composer.json that would support 7.4 through 8.2, I just did a
'composer install' from PHPv8, and then switched PHP versions out from
under it. Literally nothing broke at all - I was completely shocked! I
was so worried that I wasn't testing it right that I had to make a
quick little test script with dynamic properties in order to force the
deprecation warning to show up - and it did, so I do have the
latest.
Bad News: That's because later versions of Laravel dump all
deprecation warnings straight to /dev/null by default. If you
configure a log 'channel' for 'deprecations', then you get actual
results. One page load (our dashboard page) generated 267KB of
deprecation warnings. Clicking through most of the main pages of our
app generated around 3.5MB of them. And these aren't full
stack-traces, they're just messages like:
"[2021-12-01 03:48:23] local.WARNING: Creation of dynamic property
Illuminate\Database\MySqlConnection::$readWriteType is deprecated in
/Users/uberbrady/Documents/grokability/snipe-it/vendor/laravel/framework/src/Illuminate/Database/Connection.php
on line 1368"
Good News: Many many many of those deprecation warnings were repeats
of the same deprecated thing from the exact same place. And very very
few were from the dynamic properties thing. None of the dynamic
properties deprecations were from our code. Everything was from
composer dependencies, and only a small handful - so, probably pretty
easy to remediate when it comes to that.
So the other deprecation notices are a whole other thing, and having
so many is definitely a drag, but that's a completely separate issue
from this dynamic properties thing, which doesn't seem too bad so far,
for us at least. Other folks might not get off so easily, and
/dev/null-ing deprecation warnings seems like it's kicking the can
down the road, but, well, we're OK for 8.2.
I should note that a lot of my PHP-based CLI tools are spewing
deprecations everywhere as well, but they at least still work so I can
perhaps live with that.
-B.
…It's been merged to master, so you could stand up a build now and point to the many deprecation warnings you're expecting. I'm not saying send PRs to fix them all, just show real impact rather than theoretical guessing. …
That’s a totally fair ask, and I’ll try and get that done and report back. I think I’m probably going to run into some Laravel problems here, as they only just came up with a version that will run under php 8.1. I’ll get as far as I can and let you know - even if I determine that my suspicions were completely incorrect :)
Okay - good news/bad news/good news -
Good News: I managed to snag an 8.2 build and ran Snipe-IT on it, and
it worked completely fine. I cheated a little bit - I didn't try to
make a composer.json that would support 7.4 through 8.2, I just did a
'composer install' from PHPv8, and then switched PHP versions out from
under it. Literally nothing broke at all - I was completely shocked! I
was so worried that I wasn't testing it right that I had to make a
quick little test script with dynamic properties in order to force the
deprecation warning to show up - and it did, so I do have the
latest.Bad News: That's because later versions of Laravel dump all
deprecation warnings straight to /dev/null by default. If you
configure a log 'channel' for 'deprecations', then you get actual
results. One page load (our dashboard page) generated 267KB of
deprecation warnings. Clicking through most of the main pages of our
app generated around 3.5MB of them. And these aren't full
stack-traces, they're just messages like:"[2021-12-01 03:48:23] local.WARNING: Creation of dynamic property
Illuminate\Database\MySqlConnection::$readWriteType is deprecated in
/Users/uberbrady/Documents/grokability/snipe-it/vendor/laravel/framework/src/Illuminate/Database/Connection.php
on line 1368"Good News: Many many many of those deprecation warnings were repeats
of the same deprecated thing from the exact same place. And very very
few were from the dynamic properties thing. None of the dynamic
properties deprecations were from our code. Everything was from
composer dependencies, and only a small handful - so, probably pretty
easy to remediate when it comes to that.So the other deprecation notices are a whole other thing, and having
so many is definitely a drag, but that's a completely separate issue
from this dynamic properties thing, which doesn't seem too bad so far,
for us at least. Other folks might not get off so easily, and
/dev/null-ing deprecation warnings seems like it's kicking the can
down the road, but, well, we're OK for 8.2.I should note that a lot of my PHP-based CLI tools are spewing
deprecations everywhere as well, but they at least still work so I can
perhaps live with that.-B.
Thanks for the follow up!
Out of curiosity, since it sounds like the dynamic props deprecation rates a "meh" on the problem scale for you, what is the most common other deprecation you're hitting? It would be interesting to see how many unique deprecation warnings a large, real-world application has and compare with other projects.
--Larry Garfield
On Tue, Nov 30, 2021 at 10:18 PM Brady Wetherington <
bwetherington@grokability.com> wrote:
…It's been merged to master, so you could stand up a build now and
point to the many deprecation warnings you're expecting. I'm not saying
send PRs to fix them all, just show real impact rather than theoretical
guessing. …
Okay - good news/bad news/good news -Bad News: That's because later versions of Laravel dump all
deprecation warnings straight to /dev/null by default. If you
configure a log 'channel' for 'deprecations', then you get actual
results. One page load (our dashboard page) generated 267KB of
deprecation warnings. Clicking through most of the main pages of our
app generated around 3.5MB of them. And these aren't full
stack-traces, they're just messages like:"[2021-12-01 03:48:23] local.WARNING: Creation of dynamic property
Illuminate\Database\MySqlConnection::$readWriteType is deprecated in/Users/uberbrady/Documents/grokability/snipe-it/vendor/laravel/framework/src/Illuminate/Database/Connection.php
on line 1368"Good News: Many many many of those deprecation warnings were repeats
of the same deprecated thing from the exact same place. And very very
few were from the dynamic properties thing. None of the dynamic
properties deprecations were from our code. Everything was from
composer dependencies, and only a small handful - so, probably pretty
easy to remediate when it comes to that.
Does it look like there's any sensitive information in these log entries?
You mentioned they were only in external dependencies so I'm
thinking/hoping not. What I'm getting at is: Would you mind sharing those
raw logs somewhere. I'm sure someone will find it a fun opportunity to make
some pull requests in a variety of projects.
-Sara
…It's been merged to master, so you could stand up a build now and point to the many deprecation warnings you're expecting. I'm not saying send PRs to fix them all, just show real impact rather than theoretical guessing. …
...
Does it look like there's any sensitive information in these log entries? You mentioned they were only in external dependencies so I'm thinking/hoping not. What I'm getting at is: Would you mind sharing those raw logs somewhere. I'm sure someone will find it a fun opportunity to make some pull requests in a variety of projects.
Nope, not at all. Here I extracted out just the specific deprecations
and added counts so anyone can see how many times a deprecation
warning came up. Please do note that it is very likely that newer
versions of these libraries may have already resolved the issues, I
haven't bothered yet to try and upgrade to the latest versions.
uberbrady@GrokBook-Pro-Black snipe-it % cut -d " " -f 4-
storage/logs/deprecations.log|sort |uniq -c
10 Creation of dynamic property
Barryvdh\Debugbar\DataCollector\ViewCollector::$name is deprecated in
/Users/uberbrady/Documents/grokability/snipe-it/vendor/barryvdh/laravel-debugbar/src/DataCollector/ViewCollector.php
on line 24
10 Creation of dynamic property
Barryvdh\Debugbar\DataFormatter\QueryFormatter::$cloner is deprecated
in /Users/uberbrady/Documents/grokability/snipe-it/vendor/maximebf/debugbar/src/DebugBar/DataFormatter/DataFormatter.php
on line 23
10 Creation of dynamic property
Barryvdh\Debugbar\DataFormatter\QueryFormatter::$dumper is deprecated
in /Users/uberbrady/Documents/grokability/snipe-it/vendor/maximebf/debugbar/src/DebugBar/DataFormatter/DataFormatter.php
on line 24
30 Creation of dynamic property
Barryvdh\Debugbar\DataFormatter\SimpleFormatter::$cloner is deprecated
in /Users/uberbrady/Documents/grokability/snipe-it/vendor/maximebf/debugbar/src/DebugBar/DataFormatter/DataFormatter.php
on line 23
30 Creation of dynamic property
Barryvdh\Debugbar\DataFormatter\SimpleFormatter::$dumper is deprecated
in /Users/uberbrady/Documents/grokability/snipe-it/vendor/maximebf/debugbar/src/DebugBar/DataFormatter/DataFormatter.php
on line 24
10 Creation of dynamic property
DebugBar\DataFormatter\DataFormatter::$cloner is deprecated in
/Users/uberbrady/Documents/grokability/snipe-it/vendor/maximebf/debugbar/src/DebugBar/DataFormatter/DataFormatter.php
on line 23
10 Creation of dynamic property
DebugBar\DataFormatter\DataFormatter::$dumper is deprecated in
/Users/uberbrady/Documents/grokability/snipe-it/vendor/maximebf/debugbar/src/DebugBar/DataFormatter/DataFormatter.php
on line 24
12 Creation of dynamic property
Illuminate\Database\MySqlConnection::$readWriteType is deprecated in
/Users/uberbrady/Documents/grokability/snipe-it/vendor/laravel/framework/src/Illuminate/Database/Connection.php
on line 1368
4 class_exists()
: Passing null to parameter #1 ($class) of type
string is deprecated in
/Users/uberbrady/Documents/grokability/snipe-it/vendor/adldap2/adldap2/src/Configuration/DomainConfiguration.php
on line 153
626 mb_convert_encoding()
: Handling HTML entities via mbstring is
deprecated; use htmlspecialchars, htmlentities, or
mb_encode_numericentity/mb_decode_numericentity instead in
/Users/uberbrady/Documents/grokability/snipe-it/vendor/symfony/var-dumper/Dumper/HtmlDumper.php
on line 963
84 preg_match()
: Passing null to parameter #2 ($subject) of type
string is deprecated in
/Users/uberbrady/Documents/grokability/snipe-it/vendor/barryvdh/laravel-debugbar/src/JavascriptRenderer.php
on line 150
4 str_replace()
: Passing null to parameter #3 ($subject) of type
array|string is deprecated in
/Users/uberbrady/Documents/grokability/snipe-it/vendor/laravel/passport/src/PassportServiceProvider.php
on line 268
84 substr()
: Passing null to parameter #1 ($string) of type string
is deprecated in
/Users/uberbrady/Documents/grokability/snipe-it/vendor/barryvdh/laravel-debugbar/src/JavascriptRenderer.php
on line 150
I'm not saying send PRs to fix them all... Let's make PHP better,
together.
On a similar theme, trying to avoid too much work for developers upgrading
to later versions of PHP.
Is there any value in me proposing an RFC to update some internal
functions so they can accept NULL?
I see developers using their framework of choice for GET/POST/COOKIE/etc
values (where they receive NULL
to represent unset values), or simply doing
$q = ($_GET['q'] ?? NULL)
, and other sources... where they will now get
deprecation messages whenever they use functions like htmlspecialchars()
,
trim()
, strpos()
, strtoupper()
, strlen()
.
For example, a search page, where the search term is defined in the URL
(e.g. "/search/?q=abc"), and that value is shown on the page, often in an
<input type="search" name="q" value="?" />
for the user to edit, and
sometimes repeated in the page content (where they may use strtoupper()
for styling purposes, instead of doing that via CSS text-transform: uppercase
).
Craig
On Thu, Dec 2, 2021 at 8:48 AM Craig Francis craig@craigfrancis.co.uk
wrote:
I'm not saying send PRs to fix them all... Let's make PHP better,
together.On a similar theme, trying to avoid too much work for developers upgrading
to later versions of PHP.Is there any value in me proposing an RFC to update some internal
functions so they can accept NULL?I see developers using their framework of choice for GET/POST/COOKIE/etc
values (where they receiveNULL
to represent unset values), or simply doing
$q = ($_GET['q'] ?? NULL)
, and other sources... where they will now get
deprecation messages whenever they use functions likehtmlspecialchars()
,
trim()
,strpos()
,strtoupper()
,strlen()
.
I'm not hard against this idea. The interpretation of null in these
contexts as being equivalent to empty string isn't unreasonable. I guess
the only objection I could have would be an academic one and I can't really
defend that. So yeah, sure... why not?
I would say that such applications should consider unifying their own
types. $a = $_GET['q'] ?? ''; Is there a place in the application where
empty string and null would have been distinct? i.e. Is a search for
nothing different from not searching?
-Sara
Am 02.12.2021 um 16:19 schrieb Sara Golemon pollita@php.net:
I would say that such applications should consider unifying their own
types. $a = $_GET['q'] ?? ''; Is there a place in the application where
empty string and null would have been distinct? i.e. Is a search for
nothing different from not searching?
Being able to detect if a parameter was present and empty or it it was not present at all can be useful. That is the whole point of null, even though some people dislike null :-)
In fact we ran into the same problem and are silencing certain notices, warnings and deprecations because they get in the way of our code base. (Side-note: We are patching it instead of suppressing it in an error handler as we saw a noticeable performance difference.)
For the curious, here is our patch:
perl -0777 -i.bak -pe 's#zend_error.E_(NOTICE|WARNING|DEPRECATED).(\n.)?(Undefined|Attempt to read property|Uninitialized string offset|Trying to access array offset on value of type|Passing null to parameter)#if (0) $&#g; s#zend_type_error.must be of type Countable#if(0) $&#g' Zend/.[ch] ext/spl/.[ch] ext/opcache/jit/*.[ch]
I am not advocating that other people should do this, I'm just corroborating ;-)
Regards,
- Chris
I'm not hard against this idea. The interpretation of null in these
contexts as being equivalent to empty string isn't unreasonable. I guess
the only objection I could have would be an academic one and I can't really
defend that. So yeah, sure... why not?
Thanks, I'll start by creating a list of functions I think apply (anyone
who has any suggestions, please let me know... I'm just going to look at
some log files to get started... or try to work out if
Z_PARAM_STRING_OR_NULL
vs Z_PARAM_STR
/ Z_PARAM_STRING
can give me a
list of functions to look though).
I would say that such applications should consider unifying their own
types. $a = $_GET['q'] ?? ''; Is there a place in the application where
empty string and null would have been distinct? i.e. Is a search for
nothing different from not searching?
While I think unifying can help in some cases, there is a lot of existing
code where developers have used the null coalesce operator to return NULL
(either out of habit, or because they need to determine the difference
between an empty string vs an unset value)... the main source being the
functions frameworks have provided to return GPC values, e.g.
- Laravel: $request->input('name');
- Symfony: $request->get('name');
- CakePHP: $this->request->getQuery('name');
- CodeIgniter: $request->getGet('name');
As to the search example... I've seen one project where the search pages
would only show the form at first (when null); then, when the user did a
search, it showed the search form again, the results table (where an empty
string would show all records, useful to see the total number of records),
and present additional filtering fields (done like this for performance
reasons).
That's not to say all of these examples can't be updated to work, but there
will be a lot to change... like one project which had simply used trim()
to remove whitespace from some user inputs, where the fields/values are
normally present, but because NULL
can happen (even via odd things, like
the user being on a slow connection and the form not loading completely),
it's now a problem.
Thanks again,
Craig
I'm not hard against this idea. The interpretation of null in these
contexts as being equivalent to empty string isn't unreasonable. I guess
the only objection I could have would be an academic one and I can't really
defend that. So yeah, sure... why not?I would say that such applications should consider unifying their own
types. $a = $_GET['q'] ?? ''; Is there a place in the application where
empty string and null would have been distinct? i.e. Is a search for
nothing different from not searching?
I wonder what is the technical benefit from that?
I do feel we are moving to a strict typed language. If that is what is
desired, then let decide it clearly and move forward to rhst direction.
best,
Pierre