azjezz / psl

📚 PHP Standard Library - a modern, consistent, centralized, well-typed, non-blocking set of APIs for PHP programmers
MIT License
1.21k stars 71 forks source link

feat: introduce `first_opt()`, `first_key_opt()`, `last_opt()`, `last_key_opt()` and `search_opt()` #467

Closed simPod closed 5 months ago

simPod commented 5 months ago

I'd like to modify search operations over iterables by wrapping the result in Option.

Currently, it is not possible to use these functions to find null value since null represents "no result".

This applies to all first*, last* and search functions.

WDYT?

azjezz commented 5 months ago

I like the idea of returning an Option, but i dislike having this type of BC, what about adding new functions that result in an Option? e.g first_opt<T>(iterable<T> $i): Option<T> vs first<T>(iterable<T> $I): null|T?

_opt suffix is one way to go about this ( this is a popular choice in the rust community, e.g: https://docs.rs/chrono/latest/chrono/struct.Date.html#method.and_hms_micro VS https://docs.rs/chrono/latest/chrono/struct.Date.html#method.and_hms_micro_opt), but i'm open to suggestions if you have any, maybe we can find a more suitable naming convention for those functions.

cc @veewee this might interest you.

simPod commented 5 months ago

I'll go for it. Thanks for input.

veewee commented 5 months ago

@azjezz

The migration path might be better this way indeed. However you'll probably end up to something that looks like this in the long run, which might be something we'dd want to avoid?

azjezz commented 5 months ago

@veewee personally i would like to keep both, i don't see why we need to deprecate/remove the old behaviour.

coveralls commented 5 months ago

Pull Request Test Coverage Report for Build 8953186695

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details


Totals Coverage Status
Change from base Build 8832169884: 0.01%
Covered Lines: 4415
Relevant Lines: 4469

💛 - Coveralls