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

Replace slice methods tail(), init() with pop_first(), pop_last() #24184

Closed
wants to merge 2 commits into from

Conversation

lilyball
Copy link
Contributor

@lilyball lilyball commented Apr 8, 2015

The init/tail terminology is not obvious to anyone who hasn't
already seen it in another language like Haskell, and we already got rid
of head() (the common pair for tail()).

Instead of merely renaming these functions, turn them into separate
functions

fn pop_first(&self) -> Option<(&T, &[T])>;
fn pop_last(&self) -> Option<(&T, &[T])>;

as these functions are often used in a context where the first/last
element wants to be inspected and it's a bit more ergonomic this way.

Fixes #24141.

[breaking-change]

The `init`/`tail` terminology is not obvious to anyone who hasn't
already seen it in another language like Haskell, and we already got rid
of `head()` (the common pair for `tail()`).

Instead of merely renaming these functions, turn them into separate
functions

    fn pop_first(&self) -> Option<(&T, &[T])>;
    fn pop_last(&self) -> Option<(&T, &[T])>;

as these functions are often used in a context where the first/last
element wants to be inspected and it's a bit more ergonomic this way.
@rust-highfive
Copy link
Collaborator

r? @gankro

(rust_highfive has picked a reviewer for you, use r? to override)

@GuillaumeGomez
Copy link
Member

I prefer the first/tail naming. It makes sense from my point of view and I don't really like yours (I speak about functions naming) ^^'. I never developped in Haskell but tail doesn't "hurt" my feelings and does what I expect it to do. Why changing it then ? After, I guess it's a personal taste...

@nikomatsakis
Copy link
Contributor

Hmm, I have mixed reactions. I admire your attempt to find something categorically better than init/tail, and this may be in the right direction. And I really quite strongly dislike the name init, so I'm inclined to like anything that changes that (prefix, anybody?). But I do have some concerns:

  1. The names pop_first and pop_last seem misleading in that they do not mutate the underlying data. I'd prefer a name like split, though split currently returns two slices. Perhaps split_front_elem or divide or something like that.
  2. The change to return a tuple is nice when it works, but slice.pop_last().unwrap().1 is quite a mouthful (as compared to slice.init()). On the other hand, it's probably mostly the unwrap, which is the whole point of this PR.
  3. Note that we made a deliberate decision to have many slice methods continue to panic on invalid indices precisely because when we attempted to convert them all to return Option, it seemed by and large to just make code messier.

(Also, in collections, we tend to use the names "front" and "back", so maybe we should stick to that, rather than "first/last". Or does that seem specific to LinkedList?)

cc @alexcrichton @aturon

@nikomatsakis
Copy link
Contributor

Having said that, it seems to me that this is an alternative to #24172. I figure we might decide to take this PR, #24172, or neither. But let's move the conversation to #24141 so it can be consolidated.

@bors
Copy link
Contributor

bors commented Apr 8, 2015

☔ The latest upstream changes (presumably #23998) made this pull request unmergeable. Please resolve the merge conflicts.

@alexcrichton
Copy link
Member

This has been sitting here for awhile now, so I'm going to close this in favor of the associated RFC.

@lilyball lilyball deleted the replace-slice-tail-init branch July 26, 2015 22:19
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.

Calling .tail() on a empty slice panics
7 participants