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

Core: Reimplement CharSequenceMap to obey Map contract #11308

Conversation

findepi
Copy link
Member

@findepi findepi commented Oct 12, 2024

The previous CharSequenceMap implementation did not follow general Map contract. For example, Map.keySet() should be a live view of the keys in the map, but it was a copy. This commit reimplements the class so that it obeys the map contract, while maintaining the fundamental objective of the class, which is to compare CharSequence keys by the value of char sequences they describe.

@findepi findepi force-pushed the findepi/reimplement-charsequencemap-to-obey-map-contract-0cfe9a branch 2 times, most recently from 1073832 to 74279c3 Compare October 12, 2024 20:25
@findepi
Copy link
Member Author

findepi commented Oct 12, 2024

A probably even better fix would be to use Map<CharSequenceWrapper, V> directly and perhaps using Guava's Equivalence. Guava doesn't plan to have transparent equivalence-based maps (google/guava#576 (comment)) and I think it's a really good advice to follow.

The previous `CharSequenceMap` implementation did not follow general
`Map` contract. For example, `Map.keySet()` should be a live view of the
keys in the map, but it was a copy. This commit reimplements the class
so that it obeys the map contract, while maintaining the fundamental
objective of the class, which is to compare `CharSequence` keys by the
value of char sequences they describe.
@findepi findepi force-pushed the findepi/reimplement-charsequencemap-to-obey-map-contract-0cfe9a branch from 74279c3 to cf74397 Compare October 12, 2024 20:48
@findepi findepi marked this pull request as draft October 12, 2024 22:07
@findepi
Copy link
Member Author

findepi commented Oct 12, 2024

draft - let me explore one more possible approach

Copy link

This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the [email protected] list. Thank you for your contributions.

@github-actions github-actions bot added the stale label Nov 16, 2024
Copy link

This pull request has been closed due to lack of activity. This is not a judgement on the merit of the PR in any way. It is just a way of keeping the PR queue manageable. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.

@github-actions github-actions bot closed this Nov 24, 2024
@findepi findepi deleted the findepi/reimplement-charsequencemap-to-obey-map-contract-0cfe9a branch December 4, 2024 21:03
@findepi
Copy link
Member Author

findepi commented Dec 5, 2024

Follow-up: #11704

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant