fix(Collection::pop()): count < 1 #54340
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If $count = 0 then this method is returning two items. If count<0 then things got worse than that. I think that in this case allowing $count <1 makes no sense and either we should throw an error, or we should return an empty result. In this PR I've gone with an empty result as I can see empty results being returned by the function anyway, so I would expect end user code to understand it, so I would expect that to be the least problematic solution.
I guess there is danger merging this PR if people are accidentally passing negative numbers through and their code is right by accident, but from the evidence below I think that's a weird consideration to block this PR.
Below is a tinker. I understand cases where $count is 3 and 2, but not what we're getting if it's 0,-1 etc.
Also I would have preferred $count=1 to return a Collection, but I'm sure that boat has sailed.