Skip to content

Allow passing null as $length to slice() methods#197

Merged
rtheunissen merged 1 commit intophp-ds:masterfrom
HypeMC:fix-slice-with-null
Jul 29, 2023
Merged

Allow passing null as $length to slice() methods#197
rtheunissen merged 1 commit intophp-ds:masterfrom
HypeMC:fix-slice-with-null

Conversation

@HypeMC
Copy link
Copy Markdown
Contributor

@HypeMC HypeMC commented Jul 29, 2023 •
edited
Loading

Currently the slice() methods don't work if the value of $length is null, eg:

$vec = new \Ds\Vector();
$vec->push(1, 2, 3, 4, 5, 6, 7, 8, 9);

var_dump($vec->slice(2, null));
PHP Deprecated:  Ds\Vector::slice(): Passing null to parameter #2 ($length) of type ?int is deprecated in /test.php on line 7
PHP Stack trace:
PHP   1. {main}() /test.php:0
PHP   2. Ds\Vector->slice($index = 2, $length = NULL) /test.php:7

Deprecated: Ds\Vector::slice(): Passing null to parameter #2 ($length) of type ?int is deprecated in /test.php on line 7

Call Stack:
    0.0001     402344   1. {main}() /test.php:0
    0.0001     402608   2. Ds\Vector->slice($index = 2, $length = NULL) /test.php:7

/test.php:7:
class Ds\Vector#2 (0) {
}

This is inconsistent with how array_slice(), and by extension the polyfill work, which treat null the same as when no argument is passed: https://3v4l.org/rqsnF . It also makes using a variable harder, eg:

if ($length === null) {
    var_dump($vec->slice(2));
} else {
    var_dump($vec->slice(2, $length));
}

This PR proposes allowing passing null as the length and treating it the same as when the argument isn't passed.

Tests: php-ds/tests#23

@HypeMC HypeMC mentioned this pull request Jul 29, 2023
Merged
@rtheunissen rtheunissen merged commit f111b23 into php-ds:master Jul 29, 2023
@rtheunissen
Copy link
Copy Markdown
Member

rtheunissen commented Jul 29, 2023

@HypeMC this is great, thank you. Feels like we could simplify the parsing a bit, but this looks great. I'm looking forward to coming back to this project a bit, it's been too long.

@HypeMC HypeMC deleted the fix-slice-with-null branch July 29, 2023 05:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@HypeMC @rtheunissen