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

Is-Sorted Detection and Binary Search #34

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

CTMacUser
Copy link
Contributor

I added methods to check where a Collection stops being sorted, which forms a basis to check if an entire collection is sorted.

I added methods to perform a binary search on an already-sorted Collection. The general version returns a matching range. The search is performed in several phases, and so those phases are separated into distinct methods.

Checklist

  • I've added at least one test that validates that my change is working, if appropriate
  • I've followed the code style of the rest of the project
  • I've read the Contribution Guidelines
  • I've updated the documentation if necessary

Add method to collections to find the longest prefix that is sorted along a given predicate.  Add a variant method that checks for strictly increasing prefixes.  Add overloads to both that default the predicate to the less-than operator.
Add methods to collections to perform binary searches.  All the methods assume the collection is sorted along the given predicate, or simply in non-decreasing order for the overloads that default the predicate to the standard less-than operator.  The variants return: the quickest match, the earliest match, one past the latest match, and the index range for all matches.
@CTMacUser
Copy link
Contributor Author

When first creating the pull request, I'm not sure what the "please add '?template=new.md' to the URL to switch to the appropriate template." part in the commented instructions meant.

@xwu
Copy link
Contributor

xwu commented Oct 26, 2020

@mcekr is working on isSorted in #6.

As discussed on the forum, binary search is available as partitioningIndex: is there some use case additionally that you're trying to capture here?

Add method to collections to find the longest prefix that maintains the same value according to a given predicate Add overload that defaults the predicate to the eqaulity operator.
@natecook1000
Copy link
Member

Thanks for this contribution, @CTMacUser! I don't think these additions rise to the level of utility that would warrant inclusion in the Algorithms package at this time. As @xwu points out, the partitioningIndex method covers each of the flavors of binary search, depending on the predicate. Since things like the lower bound/upper bound are specialized use cases, it's okay to rely on the lower-level component for these.

In addition, the APIs in this and the related #37 and #38 interact with some design questions that are still pretty open, in particular around the sortedness of collections (e.g. do we want to add a SortedCollection protocol, and have some of these operations attached to that?) and some of the work under way around consumers and searchers. I'd like to continue discussing this design in the forums before adding a lot of new API around this.

@CTMacUser
Copy link
Contributor Author

As discussed on the forum, binary search is available as partitioningIndex: is there some use case additionally that you're trying to capture here?

The referenced method works with a two-partition binary search, while my design uses a three-partition binary search. The reference method assumes the collection is divided into doesn't-match and does-match and then searches for the border element. The design I used assumes the collection is divided into less-than, equivalent-to, and greater-than, and then searches for any equivalent element. I require two comparisons per round instead of one, but makes up for it by stopping at the first match instead of continuing to find the border. They're not quite the same thing.

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.

3 participants