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

return error result only from Iterator.next() #45

Closed

Conversation

mykmelez
Copy link
Contributor

This is an alternative to #44 that minimizes the number of methods that return an error result down to (almost, see below) just the Iterator.next() implementation in Iter by using std::iter::once() to create an iterator that returns an error result when a failure occurs in the Cursor.iter*() methods.

The only exception is Cursor.iter_dup_from(), which still returns an error result because I encountered a compiler panic while trying to convert it to return an iterator that returns an error result. (I'm still looking at this to figure out if it's possible to work around the failure.)

A potential downside of this approach is that it returns an Iterator trait object from the Cursor.iter*() methods, which could have runtime performance implications in theory (although I'm unsure that it does in practice).

@mykmelez
Copy link
Contributor Author

The only exception is Cursor.iter_dup_from(), which still returns an error result because I encountered a compiler panic while trying to convert it to return an iterator that returns an error result. (I'm still looking at this to figure out if it's possible to work around the failure.)

I worked around it in 58d46ec and converted Cursor.iter_dup_from() to return a boxed iterator as well, so now all Cursor.iter*() methods do so, and only Iter.next() returns a result.

@mykmelez mykmelez force-pushed the return-error-result-from-iter branch from af923dc to 1c1ff62 Compare December 19, 2018 22:21
@mykmelez
Copy link
Contributor Author

mykmelez commented Dec 19, 2018

@danburkert Recent commits on this branch included unrelated changes, so I've reset the branch to a commit that contains just this change. I also modified it, based on feedback from a colleague, to use an enum to distinguish between success and failure to retrieve an iterator, which means it no longer uses a Trait object to represent the Iter, so it shouldn't incur a runtime performance penalty.

@mykmelez
Copy link
Contributor Author

The build bustage is unrelated and fixed by #47.

@mykmelez
Copy link
Contributor Author

mykmelez commented Jan 8, 2019

Closing this in favor of the approach in #48.

@mykmelez mykmelez closed this Jan 8, 2019
@mykmelez mykmelez deleted the return-error-result-from-iter branch February 12, 2019 00:09
stuhood pushed a commit to pantsbuild/lmdb-rs that referenced this pull request Jan 25, 2022
update lmdb-rkv-sys dependency to 0.8.3 and patch version for lmdb-rkv to 0.11.4
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.

1 participant