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

Add NonEmptyVec::push #5

Open
jyn514 opened this issue Sep 3, 2020 · 7 comments
Open

Add NonEmptyVec::push #5

jyn514 opened this issue Sep 3, 2020 · 7 comments

Comments

@jyn514
Copy link

jyn514 commented Sep 3, 2020

And in general any methods that only allow adding to the vec but not removing. Otherwise it's really just a Box<NonEmptySlice> which doesn't seem to pull it's weight.

@tesuji
Copy link
Owner

tesuji commented Sep 3, 2020

Thanks for the interests.
I thought about adding NonEmptyVec::push or so but I don't particularly find the use case for it.

Also, push and insert are fallible operations, I am not sure how to cleanly handle them or just
mimic the std Vec API.

@jyn514
Copy link
Author

jyn514 commented Sep 3, 2020

When you say fallible you mean because the OS could run out of memory? It seems fine to panic in that case.

@tesuji
Copy link
Owner

tesuji commented Sep 3, 2020

And what about insert out of length ? I could return an Result<(), T> but it doesn't feel nice.

@tesuji
Copy link
Owner

tesuji commented Sep 3, 2020

How about adding a as_mut_vec -> &mut Vec? After all I don't know if the use case for push is big enough.

it is a bad idea. People could use pop on &mut Vec<T> and the whole contract by NonEmptyVec is gone.

@tesuji
Copy link
Owner

tesuji commented Sep 8, 2020

So I thought a bit and found that I need more evident about the use case for push.
If the OP has more than 10 thumps up, and there is a concrete example that push is semantically useful,
I could change my mind and add that API.

@jyn514
Copy link
Author

jyn514 commented Sep 8, 2020

I don't feel very strongly about it, it just seemed an obvious addition to the API.

@tesuji
Copy link
Owner

tesuji commented Sep 8, 2020

Yeah, the current inner implementation allows use easily adding push method to NonEmptyVec.
But just because we can does not make it through. It has to be useful unless we want APIs
that nobody uses.

Alright, back to your concern in the first post:

Otherwise it's really just a Box<NonEmptySlice> which doesn't seem to pull it's weight.

Maybe. But NonEmptyVec has inner capacity field, it should be really cheap to turn an instance of
NonEmptyVec<T> into Vec<T> and back, unlike Box<T>.

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

No branches or pull requests

2 participants