# Finding min/max of an element in a vector of tuples

I'm rewriting some code I have in Python in Rust. Python has a min(collection, [key]) function which I can use to find the item which has a min value of a particular element in a tuple. I don't see an equivalent in Rust which I think I can understand why. I've come up with this solution. It works fine but I'm wondering what refinements can be made to it if any. Thanks.

``````#[derive(Clone, Debug)]
struct Data(u32, f32, f32); // time, temp, press

impl Data {
fn get_temprature(&self) -> f32 {
self.1
}

fn _get_pressure(&self) -> f32 {
self.2
}

fn min(data: &[Data], f: fn(&Data) -> f32) -> Data {
data.iter()
.reduce(|a, b| if f(a) < f(b) { a } else { b })
.unwrap_or(&Data(0, f32::MAX, f32::MAX))
.clone()
}

fn max(data: &[Data], f: fn(&Data) -> f32) -> Data {
data.iter()
.reduce(|a, b| if f(a) > f(b) { a } else { b })
.unwrap_or(&Data(0, f32::MIN, f32::MIN))
.clone()
}
}

fn main() {
let v: Vec<Data> = vec![
Data(10, 75.0, 10000.0),
Data(11, 100.0, 12000.0),
Data(12, 25.0, 8000.0),
Data(13, 50.0, 9000.0),
];

let tn = Data::min(&v, Data::get_temprature);
println!("min: {}", tn.get_temprature());

let tx = Data::max(&v, Data::get_temprature);
println!("max: {}", tx.get_temprature());
}
``````

`Iterator::min_by_key`

Edit: Oh I see you're returning `f32` which is not `Ord`. It's because having `NaN` makes the whole comparison doesn't make sense. If you know it doesn't have any NaN, wrap the float with `NotNan`.

4 Likes

The biggest thing that pops out to me is the lack of newtypes for time, temperature, and pressure. In Rust, newtypes are pretty much free in terms of time and space consumed, and not doing so has been a 125 million dollar mistake in the past for NASA.

4 Likes

There is uom - Rust

2 Likes

It's common to have a relaxed and general version with `Ordering` callback besides the handy methods with `Ord` bound. And the float types in Rust are also provided with an inherent `Ordering` method which may or may not be what you want though.

With Iterator::min_by and f32::total_cmp,

``````-.reduce(|a, b| if f(a) < f(b) { a } else { b })
-.unwrap_or(&Data(0, f32::MAX, f32::MAX))
+.min_by(|a, b| f(a).total_cmp(&f(b)))
``````

Rust Playground

4 Likes

In your case instead of having a comment describing what the tuple elements represent, I think you should use a normal struct.

``````struct Data {
time: u32,
temp: f32,
pressure: f32,
}
``````

The reason being, as your code is written you have to remember which order your fields are in. Easy to make a mistake that way. You could accidently use the temperature field when you meant to use pressure.

3 Likes

That is a good idea. I've already made that mistake when refactoring some existing Python and Javascript code. Thanks for the link an interesting read!

Thank you. I've already made this mistake in Python. Part of the reason I did it this way is I'm de-serializing a Json request which has this format. I'll see if I can adapt something like this and get my Json deserialize code to work again.

Thanks. I did not scour the docs enough. This is the kind of thing I was looking for. I've hit a few issues. I think a reply later anticipates them. It was useful none the less to figure out how to do my own version.

Oh, yeah, that can be a problem when you're interacting with json data. Sounds like you may not have control over that part. One thing you could do is have a type that represents parsed json data, and then implement `From` for that type to convert into a more idiomatic type. It is more work, though. But the up side is if the data format changes in the future, you just would need to change code related to that intermediate type, and the rest would keep working.

Alternatively your could look at the attributes of serde, rusts equivalent parsing framework. You probably can do what you need with that, but I haven't delved any deeper than renaming variables to camel case.

``````#[derive(Clone, Debug, PartialEq, PartialOrd)]
struct Temp(f32);

impl Eq for Temp{}
impl Ord for Temp{
fn cmp(&self, other: &Self) -> std::cmp::Ordering {
self.0.total_cmp(&other.0)
// self.0.partial_cmp(&other.0).unwrap()
}
}

#[derive(Clone, Debug, PartialEq, PartialOrd)]
struct Pres(f32);

impl Eq for Pres{}
impl Ord for Pres{
fn cmp(&self, other: &Self) -> std::cmp::Ordering {
self.0.total_cmp(&other.0)
// self.0.partial_cmp(&other.0).unwrap()
}
}

#[derive(Clone, Debug)]
struct Data(u32, Temp, Pres); // time, temp, press

impl Data {
fn get_temprature(&self) -> &Temp {
&self.1
}
}

fn main() {
let v: Vec<Data> = vec![
Data(10, Temp(75.0), Pres(10000.0)),
Data(11, Temp(100.0), Pres(12000.0)),
Data(12, Temp(25.0), Pres(8000.0)),
Data(13, Temp(50.0), Pres(9000.0)),
];

let tn = v.iter().min_by_key(|d| &d.1).unwrap();
// let tn = v.iter().min_by_key(Data::get_temprature).unwrap();
println!("min: {:?}", tn.get_temprature());

let tx = v.iter().max_by_key(|d| &d.1).unwrap();
println!("max: {:?}", tx.get_temprature());
}
``````

After a bit of hacking I've tried to use the newtypes pattern and I've replaced my own custom min method by min_by_key(). I think there is a bit of duplication with the cmp function going on I'm sure could be removed. I also can't figure out how to get

``````    let tn = v.iter().min_by_key(Data::get_temprature).unwrap();
``````

to compile:

``````error[E0631]: type mismatch in function arguments
--> src/main.rs:41:34
|
27 |     fn get_temprature(&self) -> &Temp {
|     --------------------------------- found signature defined here
...
41 |     let tn = v.iter().min_by_key(Data::get_temprature).unwrap();
|                       ---------- ^^^^^^^^^^^^^^^^^^^^ expected due to this
|                       |
|                       required by a bound introduced by this call
|
= note: expected function signature `for<'a> fn(&'a &Data) -> _`
found function signature `for<'a> fn(&'a Data) -> _`
note: required by a bound in `std::iter::Iterator::min_by_key`
``````

Any ideas? Thanks.

Your closure or an analogous one is the way to do it.

``````    let tn = v.iter().min_by_key(|d| d.get_temprature()).unwrap();
``````

As for why:

`min_by_key` is designed to work on iterators over a generic `T`, which might not be a reference (or `Copy` or even `Clone`), so the closure argument takes a `&T`.

In your case, `T = &Data` and so the argument must take a `&&Data`. But `get_temprature` takes a `&Data`. So the explicit closure is there to pierce one later of indirection.

The alternative is to clone your data, but don't do that; just use the explicit closure. It's a common pattern.

2 Likes

Thank you. A little later I realised the closure did not use self and that was the problem. But when I tried to change my method to the signature suggested by the compiler I still had problems. A bit of trial and error later I got this to compile:

``````impl Data {
fn get_temprature<'a>(d: &&'a Data) -> &'a Temp {
&d.1
}
}

...

// let tn = v.iter().min_by_key(|d| &d.1).unwrap();
let tn = v.iter().min_by_key(Data::get_temprature).unwrap();
println!("min: {:?}", Data::get_temprature(&tn));
``````

But I was thinking I have no idea what I've done. Thanks for the explanation. I think I shall stick with the closure. Less code!

1 Like

You changed things so that `get_temprature` takes a `&&Data` (and is an associated function, not a method that you can call with `data.method()`). A better signature would be

``````    fn get_temprature(self: &&Self) -> &Temp
``````

(as this is still a method).

However both of those are unidiomatic. It's nice and arguably cleaner when you can use `Data::get_temprature`, but when you can't, just go with the closure instead of making your signatures weird; anyone with a bit of experience will understand what's going on.

For example this won't work with either signature; without `self`, it's no longer a method, and even with `self: &&Self`, you only get one layer of "auto-ref" (and you would need two).

``````    // yay! [compiles]
let tn = v.iter().min_by_key(Data::get_temprature).unwrap();
println!("min: {:?}", tn.get_temprature());

let data = Data(0, Temp(0.0), Pres(0.0));
// boo [does not compile] :-(
data.get_temprature();
``````
2 Likes

Thanks. It is nice to know how things work under the covers. I've decided to drop the get_.... thing since I'm already using the . syntax for tuples and I now have wrapper types around my f32s.

``````    let tn = v.iter().min_by_key(|d| &d.1).unwrap();
println!("min: {:?}", tn.1);

let tx = v.iter().max_by_key(|d| &d.1).unwrap();
println!("max: {:?}", tx.1);
``````

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.