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

append and try_append #158

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

append and try_append #158

wants to merge 1 commit into from

Conversation

jmc718
Copy link

@jmc718 jmc718 commented Feb 13, 2020

Added append and try_append with tests to implement #103

@jmc718 jmc718 requested a review from bluss February 13, 2020 21:57
@segeljakt
Copy link

Any update on this? @bluss

@bluss
Copy link
Owner

bluss commented Apr 23, 2020

Can we do this - don't provide append because it doesn't have a clear error handling story. Users can use extend if they want to do this exact operation already.

try_append is good. I think it should handle both appending arrays and arrayvecs, if possible

@burdges
Copy link

burdges commented May 10, 2020

I'd think try_extend gives a better name.

@bluss
Copy link
Owner

bluss commented May 11, 2020

Good point, but try-extend can be reserved for (Into-)iterators, and we can't handle them all efficiently in one method without an extra trait. Depending on what trait is needed to handle arrays and arrayvecs for try-append, we can decide then.

Possibly we should separate them based on error handling - appending with an array should typically be "atomic" - either the whole array fits, success, or no elements are inserted, and error. With an iterator, we'd insert as many as we can, and then error if there are elements left when we are full.

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

4 participants