Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:80325
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: softfail (pb1.pair.com: domain tekwire.net does not designate 212.27.42.2 as permitted sender)
Reply-To: <francois@tekwire.net>
To: "'Nikita Popov'" <nikita.ppv@gmail.com>
Cc: "'PHP internals'" <internals@lists.php.net>
References: <003e01d0290b$2387dd30$6a979790$@yahoo.fr>	<007801d02ba0$659b9080$30d2b180$@tekwire.net> <CAF+90c-b6KykyfUG8FmD0NkPzJPXGfJ+bOJWbGAWLrsB3ia8FA@mail.gmail.com>
In-Reply-To: <CAF+90c-b6KykyfUG8FmD0NkPzJPXGfJ+bOJWbGAWLrsB3ia8FA@mail.gmail.com>
Date: Sat, 10 Jan 2015 18:03:27 +0100
Message-ID: <000901d02cf7$5adb7360$10925a20$@tekwire.net>
MIME-Version: 1.0
Content-Type: text/plain;
	charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Thread-Index: AQJLUGZm//Pi9sQwKU1Klfwo9ikS+QIf8jOCAeK3NVGboyDTAA==
Content-Language: fr
Subject: RE: [PHP-DEV] [BugFest] Feature request #38685: str_[i]replace(): Add support for (string needle, array replace)
From: francois@tekwire.net (=?UTF-8?Q?Fran=C3=A7ois_Laupretre?=)

De : Nikita Popov [mailto:nikita.ppv@gmail.com]=20
>
> str_replace is a simple function for string replacement. It has some =
(imho also unnecessary)
> extra gimmicks to implicitly do a foreach() loop during the =
replacement. This is still relatively
> simple API-wise. But if I see this (taken from the tests) as the =
search & replacement values...

> $search=3Darray('[?]',array('(?)','d','e'),'a','R');
> $repl=3Darray(
>    array('ap(?)z[?]',"b[?](?)v",null,37,"[?]n[?]"),
>    array('a',array('b',null)),
>    array('k(?)j[?]')
>);
> ... I have a pretty hard time figuring out what this is actually =
supposed to do.

This is not a real use case, this is edge-case testing !

> At this point str_replace is working on arbitrarily-nested arrays and =
also has four options to control its behavior.
> Imho this is just too much complexity for a relatively minor use case.

Let's take this one by one :

First, this all started with two feature requests for this feature. I =
then asked on the list what people thought about it and the return what =
100% positive.

So, I started a first implementation to just allow cyclic replace with a =
loop behavior. At this time, an empty replace array was considered as an =
empty string, with no warning.

Then R. Quadling complained that we should provide a way to control what =
happens when we arrive at the end of the replace array. I hadn't done it =
because I didn't want to change the API. We finally agreed on an =
additional options arg with five possible values. I don't like the =
additional arg but I accept it because : 1. It is added as last arg and =
most people will never see it, and 2. it can be useful if we need =
additional flags in the future. For instance, if we had defined this arg =
at the beginning, it could have handled case-sensitivity, which wouldn't =
have been worse than writing a sister function. I even think now to =
define a STR_REPLACE_CASE_INSENSITIVE flag as an alternative to =
str_ireplace(). This would provide all str_[i]replace features in a =
single function (don't worry, the BC break is too huge to deprecate =
str_ireplace()).

Then, you asked for an error on empty replace array. I added that.

Then, it came to my mind that, if we accept the combination of (string =
search, array replace).we must be consistent and, as we already accept =
(array search), we must also accept ((array of arrays) replace). The =
extension to arbitrarily-nested arrays is not essential but it didn't =
cost much and allowed for a cleaner multi-layered C design. Honestly, I =
hope we keep supporting it because removing it would add complexity in =
the code but I am not sure we must document recursion support beyond 1 =
level for search / 2 levels for replace (even if side notes will quickly =
document it). I agree that supporting (array search) may be a wrong =
decision (more complexity, just for performance reasons), but it was =
made a long time ago and a lot of people use it now...

I then thought that, as I had (quite useless) recursion for =
search/replace, the least I could do was to provide the same for subject =
because, here, it is really valuable. It is a quite natural use case to =
want converting every strings in an arbitrarily-nested array and I =
imagine people will quickly use this. As before, it also brings a =
cleaner design in C code.

To summarize, I started with a rather simple feature request and was =
then pushed by external wishes. About added complexity in code, the =
summary is about 100 lines of new (cruelly missing) comments, 100 lines =
added, and 130 lines modified. The rest comes from additional tests =
(which could be simpler, ok, ok). If you consider that the design is now =
simpler to understand, this is not so bad.

So, all in all, I think that the whole balance is positive, even if =
adding an argument is never wanted.

That said, I am always open to discussion and will never tell you that =
it must remain so because I have spent *endless* hours coding it :). =
Finally, a vote will take place starting January,20, which will probably =
require 2/3 of voices (not sure because BC break is very minimal, but we =
are adding an argument...).

Sorry for posting in the same thread. I will post a new [RFC]-prefixed =
message.

If you want your arguments to be more visible to newcomers, you may post =
them in the PR. If you do it, I will then post my reply.

Regards

Fran=C3=A7ois