Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:125829
Feedback-ID: id5114917:Fastmail
Message-ID: <555c9d0c-a8b0-44cf-9206-359e5bd7430f@rwec.co.uk>
Date: Mon, 21 Oct 2024 22:07:02 +0100
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PHP-DEV] [RFC] Change behaviour of array sort functions to
 return a copy of the sorted array
To: internals@lists.php.net
References: <ZydAaayECTRNG8Ackhb0tDgYjmdFKnkW4p0ISWlLFP5My4CsJBR1KKTylrD14Ags2GUyBLoJPf68N6Z6LoVzjG-tru8YAbmi74W5xzSkJ8I=@gpb.moe>
 <y61nFg8JnfpzCsL25kyI35RiCtTm7MHW_Zn3TY81lKHkmdagnyTMk0f73FQV-xuu1aRl4401vOtG5ZbK5j5Xx4_UCkF1TJnoGNm2Aw4GlSw=@gpb.moe>
Content-Language: en-GB
In-Reply-To: <y61nFg8JnfpzCsL25kyI35RiCtTm7MHW_Zn3TY81lKHkmdagnyTMk0f73FQV-xuu1aRl4401vOtG5ZbK5j5Xx4_UCkF1TJnoGNm2Aw4GlSw=@gpb.moe>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
From: imsop.php@rwec.co.uk ("Rowan Tommins [IMSoP]")

On 21/10/2024 14:20, Gina P. Banyard wrote:
> This RFC does*NOT* change the by-ref parameter passing*NOR* the in-place sorting of these functions.
> I don't see why people are thinking this would be changed, as I only ever talk about the return value.
> But I added a sentence in the Unaffected PHP Functionality section.


I think maybe people are saying that the RFC *should* aim to do that, in 
some form.

I absolutely agree that the current design is unfortunate; but I worry 
that bugs will be introduced by people not realising that the function 
both modifies and returns its argument.

Take the classic example of why DateTimeImmutable exists:

$now = new DateTime;
$tomorrow = $now->modify('+1 day');
// $tomorrow has the expected value, but $now does not

And then compare to the proposed behaviour of sort():

$names_as_entered = explode(',', $_POST['names']);
$names_sorted = sort($names_as_entered);
// oops, we just lost our original order


If someone wants to write that code (i.e. make a sorted copy), they'd 
need to force a clone on the array somehow, e.g.

$names_as_entered = explode(',', $_POST['names']);
$names_sorted = sort(iterator_to_array($names_as_entered));

Which is probably less clear than the existing version:

$names_as_entered = explode(',', $_POST['names']);
$names_sorted = $names_as_entered;
sort($names_sorted);


The same bug can easily hide inside less obviously imperative code. This 
would remove the highest 10 numbers from a list, leaving others untouched:

$result = array_filter(
     array: $data,
     callback: fn($item) => !in_array($item, array_slice(rsort($data, 
SORT_NUMERIC), 0, 10))
);

This looks like a safe refactor for performance or readability, but 
would lose the original keys and order:

$top_10 = array_slice(rsort($data, SORT_NUMERIC), 0, 10);
$result = array_filter(
     array: $data,
     callback: fn($item) => !in_array($item, $top_10)
);

[Here's a demo: https://3v4l.org/9Nieo For anyone confused by this 
example, the first version works because the fn() closure captures $data 
by-value, effectively cloning it.]


Regards,

-- 
Rowan Tommins
[IMSoP]