rfcs icon indicating copy to clipboard operation
rfcs copied to clipboard

New sort function 'sorted()' for vec that returns a new vec for chaining.

Open johalun opened this issue 6 years ago • 17 comments

I found an old (somewhat related) issue suggesting to change sort() to return &mut self which of course can't be done since it a breaking change. I suggest instead the following:

How about a new function "sorted()", that returns a new sorted vec? When I was doing iOS programming back in the day there were usually two functions for sorting, one sorted in place and one returned a new sorted vec. It was convenient to have both options.

johalun avatar Jul 24 '19 16:07 johalun

I meant that a popular itertools crate has sorted function and we don't need to have the function in the std lib IMO. itertools sorted returns vec::IntoIter, but it can be converted into Vec without an expensive operation.

ordovicia avatar Jul 30 '19 13:07 ordovicia

Thanks for the reply. I disagree though and think it should be in std. Swift, Objective-C and Python comes to mind for other languages that have this. Probably there are more.

johalun avatar Jul 30 '19 15:07 johalun

I think we need more opinions from a wider community. As README notes, issues on this repo are not actively looked at by the Rust teams, so I recommend you to talk about the idea on internals forum or discord.

ordovicia avatar Jul 30 '19 16:07 ordovicia

can we also have one for fixed size arrays so i can do let [y1,y2] = [2,1].sort();

ta32 avatar May 01 '20 12:05 ta32

I'd personally prefer if std kept few distinctive names for expensive operations, so a method that clones an array should never be called just sorted, maybe to_owned_and_sorted. It's clearer to write let mut y = x.to_owned(); y.sort(); though.

Just do let mut y = x.clone() y.sort(); for fixed sized arrays. Also arrayvec::ArrayVec: FromIterator for other things.

I could however imagine something like

impl<T: Sized> T {
    pub fn apply<F>(self, f: F) -> S where F: FnOnce(T) -> S {
        let s = f(self); s
    }
}

except doing that breaks any code that uses apply for anything, so you'd need some trait.

burdges avatar May 01 '20 12:05 burdges

pub trait Borrow<Borrowed: ?Sized> {
    fn borrow(&self) -> &Borrowed;
    fn apply_ref(self, impl FnOnce(&Borrowed)) -> Self where Self: Sized {
        f(self.borrow());
        self
    }
}
pub trait BorrowMut<Borrowed: ?Sized>: Borrow<Borrowed> {
    fn borrow_mut(&mut self) -> &mut Borrowed;
    fn apply_mut(self, f: impl FnOnce(&mut Borrowed)) -> Self where Self: Sized {
        f(self.borrow_mut());
        self
    }
}
pub trait Clone: Sized {
    fn clone(&self) -> Self;
    fn clone_from(&mut self, source: &Self) { ... }
    fn apply_to_clone(&self, f: impl FnOnce(&mut Self)) -> Self {
        let mut s = self.clone;
        f(&mut s)
        s
    }
}

burdges avatar Jun 14 '20 00:06 burdges

I think a such basic operation should be in an ergonomic way in the stdlib.

leonardo-m avatar Jun 14 '20 05:06 leonardo-m

To bring over a comment from an earlier issue; this verbiage would be great if used consistently. That way it would be very clear what each method does without needing to look in the docs. Swift already suggests (and implements) this in its standard library: Screen Shot 2020-06-14 at 10 34 39 PM

peterkos avatar Jun 15 '20 02:06 peterkos

If you've v: [T; N] then these correctly return another [T; N]:

g(v.apply_to_clone(<&[_]>::sort_unstable))

let f = |s| s.sort_by_key(...);
g(v.apply_to_clone(f))

If you add all six sorted* methods to &[_] then you'd need to add another six to [T;N] that shadow the &[_] ones, so you're adding twelve std methods, which complicates reading the documentation.

Worse, method shadowing always creates some ambiguity that makes understanding code harder. We should discuss if shadowing might complicate refactoring code from Vec<T> to [T; N] too by adjusting how changed break. If not confident, then anything like sorted* should wait until const generics and VLAs get stabilized, but the more powerful apply_* methods avoid this complexity.

burdges avatar Jun 15 '20 09:06 burdges

I too think sorted should be in std lib. In Rust playground, repl.it and other places where I can't install packages I am forced to use mutable vectors whose names don't represent the data they contain at all times. 😢

JimLynchCodes avatar Jul 12 '20 17:07 JimLynchCodes

I too think sorted should be in std lib. In Rust playground, repl.it and other places where I can't install packages I am forced to use mutable vectors whose names don't represent the data they contain at all times. cry

Just for the record, itertools is on rust playground.

SOF3 avatar Jul 15 '20 04:07 SOF3

Would be nice to have this in the standard lib to make things possible like

[v1, v2].concat().sorted()

matthiasgeihs avatar Jan 18 '23 08:01 matthiasgeihs

[v1, v2].concat().sorted()

In the std lib of D language there's something related but with a different usage: it doesn't create a new vector, it sorts the two given arrays one after the other, viewing it like a single array.

leonardo-m avatar Jan 18 '23 09:01 leonardo-m

I think rust's concept of moving self, and allowing for it to be returned again after an operation like sorting makes standardizing methods like .sorted() uniquely neat, and would love to see it in std. Having to explicitly write a scope that takes a vec, sorts it, then returns the sorted vec, is a repeated experience that sticks out like a sore thumb to me and makes working with chained operations, similarly to iterator methods, less smooth than it should be (IMO) without itertools. Maybe I'm just using chained methods and expressions "too much" for it to be considered idiomatic rust? The part about .sort() having to be exactly like that to indicate an expensive operation doesn't quite feel adequate as the sole reason other than "it's in a popular crate." I'm not entirely sure to which extent I would like to see these declarative versions of std operations implemented, but I also feel that the reasons against this approach should be better outlined.

selvmaya avatar Jan 25 '24 02:01 selvmaya