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

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

Open
johalun opened this issue Jul 24, 2019 · 16 comments
Open
Labels
A-collections Proposals about collection APIs T-libs-api Relevant to the library API team, which will review and decide on the RFC.

Comments

@johalun
Copy link

johalun commented Jul 24, 2019

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.

@ordovicia
Copy link
Contributor

@ordovicia
Copy link
Contributor

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.

@johalun
Copy link
Author

johalun commented Jul 30, 2019

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.

@ordovicia
Copy link
Contributor

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.

@jonas-schievink jonas-schievink added A-collections Proposals about collection APIs T-libs-api Relevant to the library API team, which will review and decide on the RFC. labels Nov 17, 2019
@ta32
Copy link

ta32 commented May 1, 2020

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

@burdges
Copy link

burdges commented May 1, 2020

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
Copy link

burdges commented Jun 14, 2020

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
    }
}

@leonardo-m
Copy link

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

@peterkos
Copy link

peterkos commented Jun 15, 2020

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

@burdges
Copy link

burdges commented Jun 15, 2020

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.

@JimLynchCodes
Copy link

JimLynchCodes commented Jul 12, 2020

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. 😢

@SOF3
Copy link

SOF3 commented Jul 15, 2020

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.

@matthiasgeihs
Copy link

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

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

@leonardo-m
Copy link

[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.

@mikkelens
Copy link

mikkelens commented Jan 25, 2024

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.

@GF-Huang
Copy link

GF-Huang commented Oct 2, 2024

So? Discussed for 5 years. When will add to std?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-collections Proposals about collection APIs T-libs-api Relevant to the library API team, which will review and decide on the RFC.
Projects
None yet
Development

No branches or pull requests