Skip to content

reverse vs. rev methods on DoubleEndedIterator are confusing #11770

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

Closed
thestinger opened this issue Jan 24, 2014 · 7 comments
Closed

reverse vs. rev methods on DoubleEndedIterator are confusing #11770

thestinger opened this issue Jan 24, 2014 · 7 comments

Comments

@thestinger
Copy link
Contributor

The reverse method is an in-place reversal and the rev method flips the direction of iteration. There has to be a better naming scheme for this.

@jfager
Copy link
Contributor

jfager commented Jan 24, 2014

I feel like "reverse" is the odd duck here. It's a little surprising that the way you reverse a collection in-place is via a method on its iterator, it seems like it should be on the collection directly. Meanwhile, rev is exactly what I'd expect.

@thestinger
Copy link
Contributor Author

It's a little surprising that the way you reverse a collection in-place is via a method on its iterator, it seems like it should be on the collection directly.

I don't understand what you're saying. The reverse method on iterators doesn't reverse a container, it reverses the elements yielded by an iterator in-place:

fn main() {
    let mut xs = [1, 2, 3];
    let mut ys = [4, 5, 6];
    xs.mut_iter().chain(ys.mut_iter()).reverse_();
    println!("{:?} {:?}", xs, ys);
}

Note that the trailing underscore is a workaround for a compiler bug / language design issue. A generic implementation of a trait causes the name to be reserved on all types if the trait is in scope.

@pnkfelix
Copy link
Member

see also #10632

@pnkfelix
Copy link
Member

I idly wonder whether one way to deal with issues like this would be via a design heuristic such as "if the same concept can be implemented by both an in-place modification of the receiver and a pure operation that constructs a fresh object, then one should use a method for the in-place modification, and a separate function for the construction."

(But this is based on the assumption that people would be less likely to think that a separate function modifies its argument, which may be unfounded.)

@jfager
Copy link
Contributor

jfager commented Jan 24, 2014

The description of reverse_ in the docs is "Use an iterator to reverse a container in-place", and

let mut xs = [1, 2, 3];
xs.mut_iter().reverse_();
println!("{:?}", xs);

does just that, so hopefully you see how someone could be confused.

Anyways, I'm not saying "get rid of reverse", I'm saying that of the two use cases, rev seems far more common and so should have first dibs on the name. reverse could maybe become something like rev_swap?

@nwin
Copy link
Contributor

nwin commented Jan 27, 2014

Why not use .reversed() instead of .rev()? I think .rev() is much too cryptic (could mean revert as in resetting the iterator). Abbreviations are imo ok for such a thing like .iter(), but .rev() will be used muss less often and should be self-descriptive.

@aturon aturon added the A-libs label Jun 3, 2014
@aturon
Copy link
Member

aturon commented Jan 17, 2015

This has been addressed with the recent rename to reverse_in_place.

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

5 participants