Skip to content

Uncomment binarySearchBy overload taking a Comparator #5137

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

YoshiRulz
Copy link

@YoshiRulz YoshiRulz commented May 25, 2023

  • provided the link to the related issue(s) from YouTrack N/A
  • made a reasonable amount of changes related only to the provided issues
  • can explain changes made in the pull request
    • I was looking for this overload and noticed the comment upon jumping to the source of another overload. Turns out it's as old as the initial implementation. Having this overload in the standard library "completes the family", and it's useful to at least me. I'm rolling my own copy with the same implementation for now.
  • ran the build locally and verified new functionality
  • ran related tests locally and they passed
  • do not have merge commits in the pull request

Copy link
Member

@ilya-g ilya-g left a comment

Choose a reason for hiding this comment

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

Could you describe your use case where you needed to use this overload? This would also be useful in case we want to provide a separate sample for this overload.

@YoshiRulz
Copy link
Author

YoshiRulz commented May 31, 2023

In short, I had a sorted list whose elements were data class Domain(val raw: CharSequence, val type: TypeEnum) and I needed to get the type of the element matching an input string. CharSequence is not Comparable so I couldn't use the other overload taking a selector function. The other option I considered was populating a Map<CharSequence, Domain>, which would probably have been about as performant, though it now occurs to me that it may not have given me the comparison I was after.

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.

3 participants