Skip to content

Fix interpolation search#2

Merged
RAttab merged 1 commit intoRAttab:masterfrom
pjhades:fix/interpolation-search
Oct 12, 2017
Merged

Fix interpolation search#2
RAttab merged 1 commit intoRAttab:masterfrom
pjhades:fix/interpolation-search

Conversation

@pjhades
Copy link
Copy Markdown
Contributor

@pjhades pjhades commented Oct 10, 2017

Fix the comparison after the initial interpolation jump so that we no longer miss the target key.

Add unit tests for indexer functions.

Comment thread test/indexer_test.c Outdated
struct index *index;

rill_key_t data1[CAP] = {0, 3, 6, 9, 12, 15, 18, 21, 24, 27};
index = make_index(data1, CAP);
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

You can avoid having to create a temporary array and variable by wrapping the make_index call ina macro like so: https://github.com/RAttab/rill/blob/master/test/coder_test.c#L61

I don't recommend doing that for regular code, but for tests, it significantly reduces the boiler plate.

@RAttab
Copy link
Copy Markdown
Owner

RAttab commented Oct 11, 2017

Looks good, just need that extra macro to reduce boilerplate in the tests and we can merge this.

Thanks for the fix :)

Fix the comparison after the initial interpolation jump  so that
we no longer miss the target key.

Add unit tests for indexer functions.
@pjhades
Copy link
Copy Markdown
Contributor Author

pjhades commented Oct 12, 2017

Updated, but there's still duplication as the content of the array is specified twice, anyway the test interface is simpler than before I think.

@RAttab RAttab merged commit e074d33 into RAttab:master Oct 12, 2017
@RAttab
Copy link
Copy Markdown
Owner

RAttab commented Oct 12, 2017

All good, thanks :)

@pjhades pjhades deleted the fix/interpolation-search branch October 12, 2017 19:45
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