Skip to content

Conversation

@KaranJain21
Copy link

Adds the following methods

  • checked_remove: checks if the element at a certain index can be removed from the vector, then removes it, shifting all elements after it to the left. If it is not possible to remove, returns a RemoveError
  • remove: calls checked_remove, then unwraps
  • checked_swap_remove: checks if the element at a certain index can be removed from the vector, then removes it, replacing it with the last element. If it is not possible to remove, returns a RemoveError
  • swap_remove: calls checked_swap_remove, then unwraps

All of the above methods return the removed element (if any).

Also add a RemoveError enum, with two variants:

  1. LengthOne: to avoid removing from a NonEmpty of length 1
  2. IndexOutOfBounds: for index larger than or equal to the length of the NonEmpty

@FintanH
Copy link
Collaborator

FintanH commented Aug 8, 2025

I'm thinking about what the correct API for this is. What I'm trying to keep in mind is that the non-emptiness of the collection is what we're always trying to maintain.

With that in mind, I think I'd prefer something like:

/// Removes the element found at `index`.
///
/// If the index exists, then the element is returned, alongside the other elements as a `Vec<T>`.
///
/// If the index is out of bounds then the `NonEmpty<T>` is returned as-is.
pub fn remove(self, index: usize) -> Result<(T, Vec<T>), NonEmpty<T>> {
  /* index shenanigans */
}

Note that we cannot guarantee that we have a non-empty collection as soon as we remove, so the caller can call NonEmpty::from_vec, and have their own error types as needed.

For swap_remove, it would be something similar:

pub fn swap_remove(self, index: usize) -> Result<NonEmpty<T>, NonEmpty<T>> {
  /* index shenanigans */
}

Noting here that we can actually return a NonEmpty because we're maintaining the non-emptiness 😁

How do you feel about this? Would you be up for trying out a version of the PR that has this API?

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