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 ObservableVector::swap_remove #64

Closed
bnjbvr opened this issue Dec 3, 2024 · 1 comment
Closed

Add ObservableVector::swap_remove #64

bnjbvr opened this issue Dec 3, 2024 · 1 comment

Comments

@bnjbvr
Copy link
Collaborator

bnjbvr commented Dec 3, 2024

This would have been handy for matrix-org/matrix-rust-sdk#4346 (review). I suppose that to make the proposed fix work, we would need swap_remove() to return the index of the swapped value too (even though one could imagine it's always the index of the last element).

@jplatte
Copy link
Owner

jplatte commented Dec 19, 2024

I'm not really convinced, tbh. That PR didn't end up with something like swap_remove, the only case I've seen it used is where ordering is irrelevant, because it's faster for Vec and similar collection types.

I suppose for Vector, if it consists of a single chunk the same thing applies. But for larger instances, I think that's not the case. Also, having an extra VectorDiff variant also means that every consumer has to handle that as well. I've thought of ways around that, but I don't think there's a great solution.

@bnjbvr bnjbvr closed this as not planned Won't fix, can't repro, duplicate, stale Jan 8, 2025
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