Rename .remove() to .take() in most collection types
Nobody has claimed this yet.
- Dominant language
- Markdown
- Stars
- 6.6k
- Forks
- 1.7k
- Avg merge
- 16h 14m
- Merged PRs (30d)
- 1
Description
This is something that threw me off years ago when I spent more time than I'd care to admit looking for a Collection::take() when it was actually named Collection::remove(), more specifically because I wanted to move an object out of a container (rather than simply remove it from the container) and I was going by what syntax I knew rust used in other places to do just that. Although it's been some years, it occurred to me to file an issue today, although I may have forgotten the exact details of what types I was dealing with at the time (given that this just gets harder the more one waits).
I would like to propose that methods returning T or Option<T> rather than bool should be named Foo::take() rather than Foo::remove(). This would apply to, among others, VecDeque::remove(), LinkedList::remove(), HashMap::remove(), HashMap::remove_entry(), btree_map::OccupiedEntry, hash_map::RawOccupiedEntryMut::remove(), hash_map::RawOccupiedEntryMut::remove_entry(), Vec::remove(), and (somewhat contentiously, but only if you don't primarily think of it as a collection) String::remove().
Reasons in favor of this change:
- It is semantically clearer:
.take()implies both removing something from its current holder/owner ("taking" it away), and gaining ownership of said thing yourself, whereas.remove()only implies the former and doesn't actually differ (semantically/linguistically) from what we refer to asdrop()in the rust world. - It is more consistent.
BTreeSetandHashSetalready use.take()and most rust users are introduced toOption<T>before they ever use collections, where the nomenclature is.take()and not.remove().Option<T>,std::mem,Cell,RefCell,OnceCell,SyncOnceCellall use.take()to relinquish ownership of an/the item/entry. - It is not yet too late, as none of the mentioned types currently use
.take()for anything, so.remove()could be deprecated without any breaking changes (unless any implementIteratordirectly - which I don't think is the case - asstd::iter::Iterator::take()is a thing)
Reasons against:
- It's a lot of churn.
Possible alternatives:
- Reserve
.take()for all these types (i.e. make sure no future method is created with that name) both a) to prevent confusion, and b) so thatrustccan automatically suggest usingremove()any time someone mistypes.take()instead on one of these types.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by reviewing the listed collection entry points, including VecDeque::remove(), LinkedList::remove(), HashMap::remove(), Vec::remove(), and the occupied-entry methods, then compare them with existing take() methods on BTreeSet, HashSet, and Option. Determine whether the proposed renames and deprecations fit Rust's API conventions and RFC process; done means reaching a documented decision on the proposal.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- api, backend-api-design
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 25/100