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

Filter turns regular (lists) arrays to associative (hashmaps) array #21

Closed
nreynis opened this issue Dec 11, 2017 · 2 comments · Fixed by #31
Closed

Filter turns regular (lists) arrays to associative (hashmaps) array #21

nreynis opened this issue Dec 11, 2017 · 2 comments · Fixed by #31

Comments

@nreynis
Copy link
Contributor

nreynis commented Dec 11, 2017

When you filter an array, it may become 'sparse'. PHP deals with this situation by turning it to an associative array:
https://eval.in/916518

<?php
$a1 = ['a', 'z', 'e', 'r', 't', 'y', 'u', 'i', 'o', 'p', 'q', 's', 'd'];
$a2 = ['a', 'z', 'e', 'r', 't', 'y', 'u', 'i', 'o'];

echo json_encode(array_filter($a1, function($i){ return $i != 's'; }));
echo json_encode(array_filter($a2, function($i){ return $i != 's'; }));
{"0":"a","1":"z","2":"e","3":"r","4":"t","5":"y","6":"u","7":"i","8":"o","9":"p","10":"q","12":"d"}
["a","z","e","r","t","y","u","i","o"]

This is really disturbing and unintuitive, no other language (AFAIK) do that. I know PHP always had this permanent confusion between lists and maps, but this doesn't feel right: if I filter a list I should get a filtered list in return.

Would you consider adding a boolean argument forceList or adding a filterList method who always return array_values in the internal state ?

@florianeckerstorfer
Copy link
Member

Yes, I would add such a functionality. Since PHP can filter both maps and lists I would prefer if we add an additional function filterList(), to make the behavior more clear.

@nreynis
Copy link
Contributor Author

nreynis commented Aug 17, 2018

Another more versatile solution would be to add a values method. So you can do something like:

$chain->filter(...)->values()->array;

Also that would be more consistent with current interface which already implements a keys method.

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