8000 Small improvements for the new `SortIterator` by jdreesen · Pull Request #47 · loophp/iterators · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Small improvements for the new SortIterator #47

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

Merged
merged 1 commit into from
Sep 29, 2023
Merged

Conversation

jdreesen
Copy link
Contributor
  • Removes $iterable as class field (it's only used in the constructor)
  • Changes the visibility of the compare() method to protected (so that it is the same as in the parent class)

Follows 0c810ab

- Remove $iterable as class field (it's only used in the constructor)
- change visibility of compare() method to protected (to be the same as in the parent class)
@jdreesen jdreesen requested a review from drupol as a code owner September 29, 2023 11:28
@what-the-diff
Copy link
what-the-diff bot commented Sep 29, 2023

PR Summary

  • Changes to 'SortIterator.php'
    The permissions on some parts of the 'SortIterator.php' have been altered, which controls who or what can use them. Specifically:
    • The tool's construction parameters are now 'public', which means other pieces of code can use and modify them more freely.
    • The 'compare' method, used to distinguish or organize items, is now 'protected'. This restricts its use more than the previous 'public' status, meaning it can only be used within this piece of code or in code that extends ('inherits' from) it. This increases security and potential efficiency by limiting the improper use of the method.

Copy link
Contributor
@drupol drupol left a comment

Choose a reason for hiding this comment

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

💯 Thanks!

@drupol drupol merged commit ca05de7 into loophp:main Sep 29, 2023
@jdreesen jdreesen deleted the patch-1 branch September 29, 2023 11:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
0