6
\$\begingroup\$

As detailed in this SO question, I'm using custom logic in a BTreeSet to automatically sort a foreign data type as elements are added to the container. Note that the Data type is used from another module in the actual implementation, so I can't just implement Ord for it, necessitating this approach.

I'm concerned that the iterator pipeline I'm using in main() is too complex and that the various impls are just boilerplate for something that really boils down to a single line of actual desired logic.

Here's a Rust Playground link.

use std::collections::BTreeSet;
use std::convert::{From, Into};
use std::cmp::Ordering;
use std::ops::Deref;

// Here's the original struct...
#[derive(PartialEq, Eq, Clone, Copy, Debug)]
struct Data(usize);

impl From<usize> for Data {
    fn from(i: usize) -> Self {
        Data(i)
    }
}

// And here's the wrapper type we'll use to sort it.
#[derive(PartialEq, Eq, Debug)]
struct SortableData(Data);

impl Deref for SortableData {
    type Target = Data;
    fn deref<'a>(&'a self) -> &'a Data {
        &self.0
    }
}

impl From<Data> for SortableData {
    fn from(data: Data) -> Self {
        SortableData(data)
    }
}

impl PartialOrd for SortableData {
    fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
        Some(self.cmp(other))
    }
}

impl Ord for SortableData {
    fn cmp(&self, other: &Self) -> Ordering {
        (self.0).0.cmp(&(other.0).0)
    }
}

fn main() {
    let source: [Data; 3] = [Data(9), Data(4), Data(2)];
    let sorted: BTreeSet<SortableData> = source.iter().cloned().map(|i| i.into()).collect();
    for d in sorted.iter() {
        println!("{:?}", **d);
    }
}
\$\endgroup\$

1 Answer 1

4
\$\begingroup\$

Your code looks basically how I would write it, very nice! I do have a few tiny nits.

  1. I strongly dislike using Deref except when you have a smart pointer. I fear that using Deref to compose items is going to be the Rust equivalent of using inheritance for code reuse.

    Using Deref also can be a bit surprising, as every method is now available. Let's say that your Data struct has some more methods:

    impl Data {
        fn wow(&self) { println!("Wow") }
    }
    

    Now, your SortableData has those methods as well!

    for d in sorted.iter() {
        d.wow();
    }
    

    I much prefer to be explicit about it; I would implement Borrow instead.

    I don't believe this opinion to be 100% supported by the community, but I'm going to make my case where I can ^_^

  2. When possible, I'd implement From / Into in both directions. This doesn't always make sense, but here it does.

  3. There's no need to specify the type for source — type inference handles that just fine.

  4. You could just map the into function directly: .map(Into::into).

  5. Instead of calling iter in the for loop expression, you can just take a reference.

  6. From and Into are both part of the prelude and thus don't need to be used again.

use std::borrow::Borrow;
use std::cmp::Ordering;
use std::collections::BTreeSet;

// Here's the original struct...
#[derive(PartialEq, Eq, Clone, Copy, Debug)]
struct Data(usize);

impl From<usize> for Data {
    fn from(i: usize) -> Self {
        Data(i)
    }
}

// And here's the wrapper type we'll use to sort it.
#[derive(PartialEq, Eq, Debug)]
struct SortableData(Data);

impl From<Data> for SortableData {
    fn from(data: Data) -> Self {
        SortableData(data)
    }
}

impl From<SortableData> for Data {
    fn from(data: SortableData) -> Self {
        data.0
    }
}

impl Borrow<Data> for SortableData {
    fn borrow(&self) -> &Data {
        &self.0
    }
}

impl PartialOrd for SortableData {
    fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
        Some(self.cmp(other))
    }
}

impl Ord for SortableData {
    fn cmp(&self, other: &Self) -> Ordering {
        (self.0).0.cmp(&(other.0).0)
    }
}

fn main() {
    let source = [Data(9), Data(4), Data(2)];
    let sorted: BTreeSet<SortableData> = source.iter().cloned().map(Into::into).collect();
    for d in &sorted {
        let d: &Data = d.borrow();
        println!("{:?}", d);
    }
}
\$\endgroup\$
3
  • \$\begingroup\$ Is the impl From<SortableData> for Data legal if Data isn't my type, but is defined in another module/crate? (Then, neither the implemented trait nor the data is my own type, which I believe is disallowed.) \$\endgroup\$ Commented Dec 7, 2015 at 15:23
  • \$\begingroup\$ @thirtythreeforty yes, it is. The important thing is that the trait or one of the types must be owned by you. Since you own SortableData, you should be good to go. \$\endgroup\$ Commented Dec 7, 2015 at 15:28
  • \$\begingroup\$ Gotcha. That makes more sense and is less draconian of a restriction than I thought. \$\endgroup\$ Commented Dec 7, 2015 at 15:29

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.