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

FEATURE: Allow PositionalArraySorter to keep null values #3350

Open
wants to merge 2 commits into
base: 9.0
Choose a base branch
from

Conversation

bwaidelich
Copy link
Member

By default, the PositionalArraySorter removes all null values. This change makes this behavior an option that can be passed to the constructor:

(new PositionalArraySorter(['foo' => null]))->toArray(); // []
(new PositionalArraySorter(['foo' => null], removeNullValues: false))->toArray(); // ['foo']

Besides, this cleans up the code and tests

By default, the `PositionalArraySorter` removes all `null` values.
This change makes this behavior an _option_ that can be passed to the constructor:

```php
(new PositionalArraySorter(['foo' => null]))->toArray(); // []
(new PositionalArraySorter(['foo' => null], removeNullValues: false))->toArray(); // ['foo']
```

Besides, this cleans up the code and tests
{
$arrayKeysWithPosition = [];

foreach ($this->subject as $key => $value) {
// if the value was set to NULL it was unset and should not be used
if ($value === null) {
if ($value === null && $this->removeNullValues) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I admit that I added some code cleanups with the changes.. I just couldn't stand looking at this ancient coding style..
But trust me: Apart from making this class final and some mostly cosmetic tweaks this line is the only actual change

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ich schwöre

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

Successfully merging this pull request may close these issues.

None yet

2 participants