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

Bring merge sort and insertion sort cmp function semantics together #212

Merged
merged 1 commit into from Aug 18, 2020

Conversation

anisse
Copy link
Contributor

@anisse anisse commented Aug 17, 2020

Detailed description

Merge sort uses cmp (a, b) < 0 for its first test branch, and insertion
sort cmp (a, b) > 0 ; which means the 0 boundary goes in one case in one
branch, and in the other sort function in the other branch.

We keep the semantics of the insertion sort, it makes it possible to
support compare function that return true/false instead of a distance
between items.

Add a test that was previously failing for merge sort, but working with
insertion sort, although the function is not exposed, it only triggers
at > 43 elements.

The main difference is the test function that sends true/false instead
of a difference between two integers.

Test plan

See included test

Closing issues

N/A

Merge sort uses cmp (a, b) < 0 for its first test branch, and insertion
sort cmp (a, b) > 0 ; which means the 0 boundary goes in one case in one
branch, and in the other sort function in the other branch.

We keep the semantics of the insertion sort, it makes it possible to
support compare function that return true/false instead of a distance
between items.

Add a test that was previously failing for merge sort, but working with
insertion sort, although the function is not exposed, it only triggers
at > 43 elements.

The main difference is the test function that sends true/false instead
of a difference between two integers.
@trufae trufae merged commit a8a98e9 into radareorg:master Aug 18, 2020
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.

None yet

3 participants