Thoughts on a free `cmp` function

libs
1

TLDR: I'd like a function in std like cmp(a, b) that calls a.cmp(b).

Recently, std::iter::zip was added for better readability/formatting when using iterators that are the result of lots of function calls. For example:

let a = vec![1, 2, 3].into_iter();
let b = vec![1, 2, 3].into_iter();

let zipped = a
    .map(|i| i * 2)
    .filter(|i| i % 3 == 0)
    .zip(b.map(|i| i * 2).filter(|i| i % 3 == 0));

// compared to

zip(
    a.map(|i| i * 2).filter(|i| i % 3 == 0),
    b.map(|i| i * 2).filter(|i| i % 3 == 0),
)

Recently I found myself sorting a lot of slices using .sort_by(), and this lead to a pretty similar situation with code like:

items.sort_by(|a, b| {
    a.config
        .properties
        .some_value
        .cmp(&b.config.properties.some_value)
});

However, with this cmp function I was able to get similar readability improvements as with zip.

fn cmp<T>(a: &T, b: &T) -> Ordering where T: Ord {
    a.cmp(b)
}

items.sort_by(|a, b| {
    cmp(
        &a.config.properties.some_value,
        &b.config.properties.some_value,
    )
})

There's probably scope for an equivalent for partial_cmp, though I suspect the "symmetry" benefits are less obvious if the two arguments might have different types :person_shrugging:

I'd be keen to see this in the standard library, and if other people also think it would be a good addition I'd be happy to open a PR for it. But of course if there are reasons why such a function isn't needed, or doesn't belong in std, I'd be more than happy to hear them too.

Thanks :grin:

3 Likes
2

It's worth mentioning that you can approximate this by calling Ord::cmp:

use std::cmp::Ord;

items.sort_by(|a, b| {
    Ord::cmp(
        &a.config.properties.some_value,
        &b.config.properties.some_value,
    )
})

But you can't do use std::cmp::Ord::cmp, so there could still be an argument made for having a free function that does this.

13 Likes
3

Maybe this should be allowed?

10 Likes
4

Ord is in the prelude, so you normally don't need to import it. But if you must use the path, you can write the call in full: <_ as std::cmp::Ord>::cmp

I feel like there was an RFC to use methods, but I can't find it now...

edit: found an issue: rust-lang/rfcs#1995

6 Likes
5 
items.sort_by(|a, b| {
    cmp(
        &a.config.properties.some_value,
        &b.config.properties.some_value,
    )
})

What others said about Ord::cmp; also if your use case is literally like this where you're just doing a map/projection and calling cmp, the sort_by_key method is a perfect fit and saves you from potential copypaste bugs:

items.sort_by_key(|a| a.config.properties.some_value)

(although there's the problem that you can't return by reference from the key function due to lifetime issues :frowning: )

4 Likes
6

As Jules-Bertholet said, I'd love to see a proposal to allow this. This isn't the only kind of thing where it'd be nice -- https://doc.rust-lang.org/nightly/std/default/fn.default.html is another thing that should just be use Default::default; instead of needing a function.

There are some issues to work through, but I think many of them are the same as already exist for things like use Result::Ok;, so hopefully wouldn't be too bad.

(zip is a bit different here, since iter::zip takes IntoIterator on both sides, and thus zip([1, 2, 3], 10..) works even though [1, 2, 3].zip(10..) doesn't.)

14 Likes
7

I think for now, it's definitely easier to just stick to using Ord::cmp(), sort_by_key IIRC doesn't work well for String (I remember running into lifetime issues a while ago and stumbled upon an issue that said it would need GATs to work with non-Copy types, but I might me misremembering).

Thanks for the suggestions :slight_smile:

8

Is there any progress made on this RFC? It seems that it would keep modules clean from free functions like default() which just internally calls some other function.

9

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.