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.

Sign up for GitHub

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

Jump to bottom

feat(collections): introduce Set, SetInterface, MutableSet, and MutableSetInterface #482

Merged
merged 1 commit into from
Jul 17, 2024
Merged

feat(collections): introduce Set, SetInterface, MutableSet, and MutableSetInterface #482

merged 1 commit into from
Jul 17, 2024

Conversation

azjezz
Copy link
Owner

@azjezz azjezz commented Jul 1, 2024

introduce the following new types:

  • Set
  • MutableSet
  • SetInterface
  • MutableSetInterface

map and mapWithKeys methods have been removed from collection interfaces, they are now present only in Map, MutableMap, MapInterface, MutableMapInterface, Vector, MutableVector, VectorInterface, MutableVectorInterface, Set, MutableSet, SetInterface, and MutableSetInterface. ( the reason is that both map and vector map using the signature map<U>(c: f(V): U) -> Self<K, U> while sets map using map<U>(c: f(V): U) -> Self<U, U> signature ).

@azjezz azjezz self-assigned this Jul 1, 2024
@azjezz azjezz added Priority: Critical This should be dealt with ASAP. Not fixing this issue would be a serious error. Status: In Progress This issue is being worked on, and has someone assigned. Type: Enhancement Most issues will probably ask for additions or changes. Type: BC Break A change that will result in a backward compatibility break in the public API. labels Jul 1, 2024
@azjezz azjezz requested a review from veewee July 1, 2024 22:18
@azjezz
Copy link
Owner Author

azjezz commented Jul 1, 2024

@veewee i need Set/MutableSet in another project, hope the change regarding the other interfaces does not break anything for you :)

@azjezz azjezz force-pushed the feat/v3-collections branch 6 times, most recently from c4ea842 to 03c9ed2 Compare July 2, 2024 13:44
@azjezz
Copy link
Owner Author

azjezz commented Jul 2, 2024

note: this change will be part of v3, but in v4, I plan on removing the current Psl\Collection component, and replacing it with a new implementation that is more rust-ish ( see https://doc.rust-lang.org/std/collections/index.html ) that makes use of Psl\Option and Psl\Result, as the current implementation was initially supposed to be ( a sort of still is )
a PHP port HH\ collections in HHVM ( https://docs.hhvm.com/hack/arrays-and-collections/object-collections )

@coveralls
Copy link

coveralls commented Jul 2, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 9762814914

Details

  • 208 of 208 (100.0%) changed or added relevant lines in 8 files are covered.
  • 2 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+0.07%) to 97.339%
Files with Coverage Reduction New Missed Lines %
src/Psl/DateTime/Internal/high_resolution_time.php 2 80.95%
Totals Coverage Status
Change from base Build 9562221483: 0.07%
Covered Lines: 5231
Relevant Lines: 5374
💛 - Coveralls

Repository owner deleted a comment from coveralls Jul 2, 2024
Repository owner deleted a comment from coveralls Jul 2, 2024
Repository owner deleted a comment from coveralls Jul 2, 2024
Repository owner deleted a comment from coveralls Jul 2, 2024
Repository owner deleted a comment from coveralls Jul 2, 2024
joshrai
joshrai reviewed Jul 3, 2024
View reviewed changes
Copy link

@joshrai joshrai left a comment

Choose a reason for hiding this comment

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

Looks good! Just left a few minor comments.

src/Psl/Collection/MutableAccessibleCollectionInterface.php Outdated Show resolved Hide resolved
src/Psl/Collection/MutableSet.php Outdated Show resolved Hide resolved
src/Psl/Collection/MutableSet.php Show resolved Hide resolved
src/Psl/Collection/SetInterface.php Outdated Show resolved Hide resolved
tests/unit/Collection/AbstractSetTest.php Outdated Show resolved Hide resolved
tests/unit/Collection/AbstractSetTest.php Show resolved Hide resolved
veewee
veewee reviewed Jul 4, 2024
View reviewed changes
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 like a good place to start from.
I don't think the changes will break anything from my end, since I'm just using the provided classes instead of interfaces.

Added some comments here and there.

src/Psl/Collection/SetInterface.php Show resolved Hide resolved
src/Psl/Type/vector.php Outdated Show resolved Hide resolved
src/Psl/Type/Internal/MutableSetType.php Outdated Show resolved Hide resolved
src/Psl/Type/Internal/MutableSetType.php Outdated Show resolved Hide resolved
src/Psl/Type/Internal/MutableSetType.php Outdated Show resolved Hide resolved
src/Psl/Collection/SetInterface.php Outdated Show resolved Hide resolved
src/Psl/Collection/SetInterface.php Show resolved Hide resolved
@coveralls
Copy link

coveralls commented Jul 11, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 9977328873

Details

  • 207 of 217 (95.39%) changed or added relevant lines in 8 files are covered.
  • 2 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-0.1%) to 97.158%
Changes Missing Coverage Covered Lines Changed/Added Lines %
src/Psl/Type/Internal/MutableSetType.php 38 39 97.44%
src/Psl/Type/Internal/SetType.php 38 39 97.44%
src/Psl/Collection/MutableSet.php 62 66 93.94%
src/Psl/Collection/Set.php 40 44 90.91%
Files with Coverage Reduction New Missed Lines %
src/Psl/DateTime/Internal/high_resolution_time.php 2 80.95%
Totals Coverage Status
Change from base Build 9562221483: -0.1%
Covered Lines: 5230
Relevant Lines: 5383
💛 - Coveralls

veewee
veewee reviewed Jul 11, 2024
View reviewed changes
src/Psl/Type/mutable_set.php Outdated Show resolved Hide resolved
src/Psl/Type/set.php Outdated Show resolved Hide resolved
@azjezz azjezz merged commit b899ad0 into next Jul 17, 2024
24 of 26 checks passed
@azjezz azjezz deleted the feat/v3-collections branch July 17, 2024 15:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joshrai joshrai joshrai left review comments

@veewee veewee veewee approved these changes

Assignees

@azjezz azjezz

Labels
Priority: Critical This should be dealt with ASAP. Not fixing this issue would be a serious error. Status: In Progress This issue is being worked on, and has someone assigned. Type: BC Break A change that will result in a backward compatibility break in the public API. Type: Enhancement Most issues will probably ask for additions or changes.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@azjezz @coveralls @joshrai @veewee