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

feat(option): introduce option component #356

Merged
merged 1 commit into from
Nov 26, 2022
Merged

feat(option): introduce option component #356

merged 1 commit into from
Nov 26, 2022

Conversation

azjezz
Copy link
Owner

@azjezz azjezz commented Jul 2, 2022

Signed-off-by: azjezz azjezz@protonmail.com

@azjezz azjezz added Priority: Medium This issue may be useful, and needs some attention. Status: In Progress This issue is being worked on, and has someone assigned. Status: Review Needed The issue has a PR attached to it which needs to be reviewed. Type: Enhancement Most issues will probably ask for additions or changes. Experimental labels Jul 2, 2022
@azjezz azjezz requested a review from veewee July 2, 2022 09:02
@azjezz azjezz self-assigned this Jul 2, 2022
@coveralls
Copy link

coveralls commented Jul 2, 2022

Pull Request Test Coverage Report for Build 3554950291

  • 37 of 37 (100.0%) changed or added relevant lines in 3 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.04%) to 98.977%

Totals Coverage Status
Change from base Build 3554915568: 0.04%
Covered Lines: 3386
Relevant Lines: 3421

💛 - Coveralls

@azjezz azjezz force-pushed the feat/option branch 2 times, most recently from 5c83ad5 to be439b8 Compare July 2, 2022 09:11
Copy link
Collaborator

@veewee veewee left a comment

Choose a reason for hiding this comment

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

Looks nice!

src/Psl/Option/Option.php Show resolved Hide resolved
src/Psl/Option/Option.php Show resolved Hide resolved
src/Psl/Option/Exception/NoneException.php Show resolved Hide resolved
@Ocramius
Copy link
Contributor

Just cross-linking here: I've provided a full example @ https://gist.github.com/Ocramius/368bd78c38cc5bb857e931801f6e4af7

Beware that Optional is not necessarily OptionalField.

  • Optional in Java attempted to abstract T|null (because Java sucks at handling null - it's so so bad!).
  • in my example, I attempt to abstract T|null|absent, where absent, which is a slightly different problem 👍

@azjezz
Copy link
Owner Author

azjezz commented Oct 14, 2022

forPossiblyMissingArrayKey

I was thinking with the addition of Option, we could add the following functions to Dict and Vec components, which would implement basically that.

Dict\get<Tk, Tv>(dict<Tk, Tv> $d, Tk $k): Option<Tv> {
  return Iter\contains_key($d, $k) ? Option\some($d[$k]) : Option\none();
}

Dict\get_typed<T>(dict<array-key, mixed> $d, array-key $k, Type\TypeInterface<T> $t): Option<T> {
  return Iter\contains_key($d, $k) ? Option\some($t->coerce($d[$k])) : Option\none();
}

Vec\get<T>(vec<T> $v, int $k): Option<T> {
  return Iter\contains_key($v, $k) ? Option\some($v[$k]) : Option\none();
}

Vec\get_typed<T>(vec<mixed> $v, int $k, Type\TypeInterface<T> $t): Option<T> {
  return Iter\contains_key($v, $k) ? Option\some($t->coerce($v[$k])) : Option\none();
}

@azjezz
Copy link
Owner Author

azjezz commented Oct 14, 2022

in my example, I attempt to abstract T|null|absent, where absent, which is a slightly different problem 👍

To note, null is a valid "Some" in this implementation of Option, where Option\some(null)->isNone() is false, it's up to you to create a "none" in case of null if that is what you desire.

@veewee veewee changed the base branch from 2.1.x to 2.2.x November 26, 2022 18:37
Signed-off-by: azjezz <azjezz@protonmail.com>
@veewee
Copy link
Collaborator

veewee commented Nov 26, 2022

Discussed the open topics in this PR with @azjezz. There is nothing blocking that is stopping us from merging and releasing this PR. We can always take in additional improvements once people start using this component and start seeing better ways to do things.

@veewee veewee added Status: Accepted It's clear what the subject of the issue is about, and what the resolution should be. Status: Completed Nothing further to be done with this issue. Awaiting to be closed by the requestor out of politeness and removed Status: In Progress This issue is being worked on, and has someone assigned. Status: Review Needed The issue has a PR attached to it which needs to be reviewed. labels Nov 26, 2022
@veewee veewee merged commit b239bb7 into 2.2.x Nov 26, 2022
@veewee veewee added this to the 2.2.0 milestone Nov 26, 2022
@azjezz azjezz deleted the feat/option branch December 12, 2022 19:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: Medium This issue may be useful, and needs some attention. Status: Accepted It's clear what the subject of the issue is about, and what the resolution should be. Status: Completed Nothing further to be done with this issue. Awaiting to be closed by the requestor out of politeness Type: Enhancement Most issues will probably ask for additions or changes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants