patternrustMinor
Using a custom comparison function for BTreeSet and a foreign type
Viewed 0 times
functionforeigncomparisontypecustomforusingandbtreeset
Problem
As detailed in this SO question, I'm using custom logic in a
I'm concerned that the iterator pipeline I'm using in
Here's a Rust Playground link.
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 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 self) -> &'a Data {
&self.0
}
}
impl From for SortableData {
fn from(data: Data) -> Self {
SortableData(data)
}
}
impl PartialOrd for SortableData {
fn partial_cmp(&self, other: &Self) -> Option {
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 = source.iter().cloned().map(|i| i.into()).collect();
for d in sorted.iter() {
println!("{:?}", **d);
}
}Solution
Your code looks basically how I would write it, very nice! I do have a few tiny nits.
-
I strongly dislike using
Using
Now, your
I much prefer to be explicit about it; I would implement
I don't believe this opinion to be 100% supported by the community, but I'm going to make my case where I can ^_^
-
When possible, I'd implement
-
There's no need to specify the type for
-
You could just map the
-
Instead of calling
-
-
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 ^_^
-
When possible, I'd implement
From / Into in both directions. This doesn't always make sense, but here it does.-
There's no need to specify the type for
source — type inference handles that just fine.-
You could just map the
into function directly: .map(Into::into).-
Instead of calling
iter in the for loop expression, you can just take a reference.-
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 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 for SortableData {
fn from(data: Data) -> Self {
SortableData(data)
}
}
impl From for Data {
fn from(data: SortableData) -> Self {
data.0
}
}
impl Borrow for SortableData {
fn borrow(&self) -> &Data {
&self.0
}
}
impl PartialOrd for SortableData {
fn partial_cmp(&self, other: &Self) -> Option {
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 = source.iter().cloned().map(Into::into).collect();
for d in &sorted {
let d: &Data = d.borrow();
println!("{:?}", d);
}
}Code Snippets
impl Data {
fn wow(&self) { println!("Wow") }
}for d in sorted.iter() {
d.wow();
}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);
}
}Context
StackExchange Code Review Q#113010, answer score: 4
Revisions (0)
No revisions yet.