Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Array Subset does not work as expected on indexed / flat arrays #3101

Closed
mrhn opened this issue Apr 22, 2018 · 1 comment · Fixed by #3161
Closed

Array Subset does not work as expected on indexed / flat arrays #3101

mrhn opened this issue Apr 22, 2018 · 1 comment · Fixed by #3161

Comments

@mrhn
Copy link

mrhn commented Apr 22, 2018

Q A
PHPUnit version 7.1.4
PHP version 7.1.16
Installation Method Composer

Digging up a older issue which i do not feel is explained properly, see #2781 and #2069.

In regards to the comment you wrote, "is it fixed in #2237?", no it is not. The the usage of array_replace_recursive and how indexed arrays work in PHP is the problem here. This seem to confuse a lot of people me included. If it works as intended is up for debate, but i will try to explain the confusion.

In set theories the set {1,2,3} will have the subset {2,3}. If we use array_replace_recursive with this. It will use the array [0 => 1, 1 => 2, 2 => 3] and replace the keys with the subset [0 => 2, 1 => 3], which for this usage of array_replace_recursive should instead be [1 => 2, 2 => 3], to make {2,3} pass as a subset, with the way the current code is written. Yes i agree, that arrays are not sets, but i feel this is how people logically assert these things in their minds. So replacing array_replace_recursive on indexed arrays would solve this as the implementer @marcioAlmada of the subset class has suggested in @2069.

@mrhn mrhn changed the title Array Subset does not as expected on indexed / flat arrays Array Subset does not work as expected on indexed / flat arrays Apr 22, 2018
@pfrenssen
Copy link
Contributor

@marcioAlmada has explained in #2069 (comment) that the current implementation of ArraySubset is only intended to be used with associative arrays. He proposed to use array_intersect() for indexed arrays in that comment.

I have tried this but it doesn't work as expected since array_intersect() will always do a string comparison on the array contents. It does not support strict comparisons or nested arrays.

Another solution is to recurse over the data set and compare values directly. I've proposed a fix in #3161 which supports both flat and multidimensional indexed arrays, and is fully backwards compatible with the current implementation for associative arrays. All the original tests are untouched and passing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants