Comparison on a `Box<(dyn Valueable + 'static)>` fails

help
1

Should I use a alternative to .cmp(), if so what would be the appropiate tool. I can't seem to figure this out.

Code

use std::cmp::Ordering;

struct Painting {
    artist: String,
    title: String,
    current_price: f32,
}

struct Car {
    brand: String,
    model: String,
    original_price: f32,
    age: u8,
}


pub trait Valueable {
    fn get_price(self) -> f32;
    fn get_description(self) -> String;
}

impl Valueable for Car {
    fn get_price(self) -> f32 {
         return self.original_price * 0.95_f32.powf(self.age.into());
    }
    fn get_description(self) -> String {
        return self.model;
    }
}

impl Valueable for Painting {
    fn get_price(self) -> f32 {
        return self.current_price;
    }
    fn get_description(self) -> String {
        return self.title;
    }
}


pub enum BST {
    Leaf {
        value: Box<dyn Valueable>,
        left: Box<BST>,
        right: Box<BST>,
    },
    Empty,
}



impl BST {
    pub fn new() -> Self {
        BST::Empty
    }

    // TODO: Replace .cmp() with a different comparison method potentially
    pub fn add(&mut self, new_value: Box<dyn Valueable>) {
        match self {
            BST::Leaf {
                value,
                left,
                right,
            } => match new_value.get_price().cmp(value.get_price()) {
            // Tried: Original: match new_value.cmp(value)
            // Tried: match new_value.get_price().cmp(&value.get_price()
            // Tried: new_value.get_price() == value.get_price()
            Ordering::Less => left.add(new_value),
            Ordering::Greater => right.add(new_value),
            Ordering::Equal => return,
        },
        BST::Empty => {
            *self = BST::Leaf {
                value: new_value,
                left: Box::new(BST::Empty),
                right: Box::new(BST::Empty),
            }
        }
    }
    }
 
    pub fn contains(&self, find_value: Box<dyn Valueable>) -> bool {
        match self {
            BST::Leaf {
                value,
                left,
                right,
            } => match find_value.cmp(value) {
                Ordering::Less => left.contains(find_value),
                Ordering::Greater => right.contains(find_value),
                Ordering::Equal => true,
            },
            BST::Empty => false,
        }
    }

    pub fn as_vector(&mut self) -> Vec::<String> {
        match self {
            BST::Empty => return Vec::new(),
            BST::Leaf { value, left ,right } => {
                let mut values = left.as_vector();
                values.push(value.clone());
                values.extend(right.as_vector());
                values
            },
        }
    }
}


fn main(){
    let mut bst = BST::new();

    let car1 = Car {
        brand: "Volvo".to_string(),
        model: "123ABC".to_string(),
        original_price: 23000.50,
        age: 12,
    };
    BST::add(&mut bst, Box::new(car1));

    let painting1 = Painting {
        artist: "Hans".to_string(),
        title: "Hanswacht".to_string(),
        current_price: 999.33,
    };
}

Compiler output

error[E0599]: `f32` is not an iterator
  --> main.rs:66:46
   |
66 |             } => match new_value.get_price().cmp(value.get_price()) {
   |                                              ^^^ `f32` is not an iterator
   |
   = note: the following trait bounds were not satisfied:
           `f32: Iterator`
           which is required by `&mut f32: Iterator`

error[E0599]: the method `cmp` exists for struct `Box<(dyn Valueable + 'static)>`, but its trait bounds were not satisfied
  --> main.rs:91:35
   |
17 | pub trait Valueable {
   | ------------------- doesn't satisfy `(dyn Valueable + 'static): Iterator`, `dyn Valueable: Iterator` or `dyn Valueable: Ord`
...
91 |             } => match find_value.cmp(value) {
   |                                   ^^^ method cannot be called on `Box<(dyn Valueable + 'static)>` due to unsatisfied trait bounds
   |
   = note: the following trait bounds were not satisfied:
           `dyn Valueable: Iterator`
           which is required by `Box<dyn Valueable>: Iterator`
           `dyn Valueable: Ord`
           which is required by `Box<dyn Valueable>: Ord`
           `Box<(dyn Valueable + 'static)>: Iterator`
           which is required by `&mut Box<(dyn Valueable + 'static)>: Iterator`
           `(dyn Valueable + 'static): Iterator`
           which is required by `&mut (dyn Valueable + 'static): Iterator`
   = help: items from traits can only be used if the trait is implemented and in scope
   = note: the following traits define an item `cmp`, perhaps you need to implement one of them:
           candidate #1: `Ord`
   = note: the trait `Iterator` defines an item `cmp`, but is explicitly unimplemented

error[E0599]: the method `clone` exists for mutable reference `&mut Box<(dyn Valueable + 'static)>`, but its trait bounds were not satisfied
   --> main.rs:106:35
    |
 17 | pub trait Valueable {
    | ------------------- doesn't satisfy `dyn Valueable: Clone` or `dyn Valueable: Sized`
...
106 |                 values.push(value.clone());
    |                                   ^^^^^ method cannot be called on `&mut Box<(dyn Valueable + 'static)>` due to unsatisfied trait bounds
    |
    = note: the following trait bounds were not satisfied:
            `dyn Valueable: Sized`
            which is required by `Box<dyn Valueable>: Clone`
            `dyn Valueable: Clone`
            which is required by `Box<dyn Valueable>: Clone`
    = help: items from traits can only be used if the trait is implemented and in scope
    = note: the trait `Clone` defines an item `clone`, but is explicitly unimplemented

warning: unused variable: `painting1`
   --> main.rs:126:9
    |
126 |     let painting1 = Painting {
    |         ^^^^^^^^^ help: if this is intentional, prefix it with an underscore: `_painting1`
    |
    = note: `#[warn(unused_variables)]` (part of `#[warn(unused)]`) on by default

For more information about this error, try `rustc --explain E0599`.
warning: `src` (bin "src") generated 1 warning
error: could not compile `src` (bin "src") due to 3 previous errors; 1 warning emitted
2

it looks like you are pretty new, because your code doesn't make a lot of sense.

firstly, you are clearly trying to do things OOP style, which is strongly recommended against in general in rust, as it is particularly hard.
dyn Trait are a pretty complex topic that can be pretty hard to work with.

for something like what you have right now, an enum would be more recommened.

but you can still make it work with dyn Trait if you really want to.

next your Valueable trait is pretty wrong

pub trait Valueable {
    fn get_price(self) -> f32;
    fn get_description(self) -> String;
}

taking self consume the value, destroying it. you do not want to destroy a Valueable just to get its price.
instead you should take &self.
for the description, same thing. you could reurn String by cloning String, but it is much better to just return &str and let the caller decide what they need.
also we generally don't use get_property in rust, just property(). so you should end up with

pub trait Valueable {
    fn price(&self) -> f32;
    fn description(&self) -> &str;
}

next, yoour first true error :

new_value.get_price().cmp(value.get_price())

this doesn't work.
f32s, which are floats, have the property that NaN != NaN(this is the float standard that is true in all languages), that means you cannot call cmp on f32 because there is no ordering for comparing to NaNs.
there is f32::total_cmp for comparing them regardless, but without respecting the usual rules (here nans can equal each other).

next error :

find_value.cmp(value)

it is really not clear what you want to do here. you never wrote any code that tells the compiler how it is supposed to compare a Painting with a Car. you need to decide how you want it to work first, then you can implement it.

finally:

values.push(value.clone());

i'm not sure what you are trying to do.
you return a Vec<String>, but value is a Box<dyn Valueable>. what do you want the String to be ?

3

Thank you for the extensive and fast response first of all!

dyn Trait are a pretty complex topic that can be pretty hard to work with. for something like what you have right now, an enum would be more recommened. firstly, you are clearly trying to do things OOP style, which is strongly recommended against in general in rust, as it is particularly hard.

I am doing a practice assignment, which structured it out like this. I will keep it in mind for in the future though.

dyn Trait are a pretty complex topic that can be pretty hard to work with. for something like what you have right now, an enum would be more recommened. but you can still make it work with dyn Trait if you really want to.

That makes sense, in my case I am deliberately practicing with trait objects to understand it better.

next your Valueable trait is pretty wrong. Taking self consume the value, destroying it. you do not want to destroy a Valueable just to get its price. instead you should take &self. for the description, same thing. you could reurn String by cloning String, but it is much better to just return &str and let the caller decide what they need.

That makes sense I did not completly understand how & works, but this explained it well for me. And the difference between String and &str.

also we generally don't use get_property in rust, just property(). so you should end up with

Okay, I did not know. Online I could also not find about it, but it is cleaner.

next, yoour first true error. this doesn't work. f32s, which are floats, have the property that NaN != NaN(this is the float standard that is true in all languages), that means you cannot call cmp on f32 because there is no ordering for comparing to NaNs.
there is f32::total_cmp for comparing them regardless, but without respecting the usual rules (here nans can equal each other).

I want to sort the prices descendingly for both paintings and cars using a binary search tree. So f32::total_cmp could be a good fit here scince negative prices would not occur right?

it is really not clear what you want to do here. you never wrote any code that tells the compiler how it is supposed to compare a Painting with a Car. you need to decide how you want it to work first, then you can implement it.

It should compare the prices of the paintings and cars to check if they are equal I think. I created another binary search tree which I used as a base to compare strings for reference. Should I implement a iterator? Or could I do it simpler without using match and using == for example?

impl BST { pub fn new() -> Self { BST::Empty } 
pub fn add(&mut self, new_value: String){
    match self {
        BST::Leaf {
            value,
            left,
            right,
        } => match new_value.cmp(value) {
            Ordering::Less => left.add(new_value),
            Ordering::Greater => right.add(new_value),
            Ordering::Equal => return,
        },
        BST::Empty => {
            *self = BST::Leaf {
                value: new_value,
                left: Box::new(BST::Empty),
                right: Box::new(BST::Empty),
            }
        }
    }
}

i'm not sure what you are trying to do. you return a Vec<String>, but value is a Box<dyn Valueable>. what do you want the String to be ?

I want to return a list of Valueables (calling .to_string() on each Valueable) to print their list of descriptions and current prices. I was not completly done with the other functions yet, sorry for the confustion.

4

cmp() is a method of the Ord trait, which requires the implementing type to have a total order, but floating point numbers is only partially ordered, i.e. they implementes PartialOrd, not Ord, which means you cannot use cmp() on floating point types such as f32

use .partial_cmp() instead, which is from PartialOrd. or alternatively, use an if-else chain with the comparison operators like <, ==, >.

5

total_cmp can handles negative values just fine and the same as regular comparison.
it's only NaNs which are different.

that would mean that a car and a painting of the same price would be considered equal, which is quite weird, but if that it what you want to do you can simply write find_value.price().total_cmp(&value.price()).

6

I made some changes.

Code

use std::cmp::Ordering;

#[derive(Clone)]
struct Painting {
    artist: String,
    title: String,
    current_price: f32,
}

#[derive(Clone)]
struct Car {
    brand: String,
    model: String,
    original_price: f32,
    age: u8,
}

pub trait Valueable {
    fn price(&self) -> f32;
    fn description(&self) -> &str;
}

impl Valueable for Car {
    fn price(&self) -> f32 {
         return self.original_price * 0.95_f32.powf(self.age.into());
    }
    fn description(&self) -> &str {
        return &self.model;
    }
}

impl Valueable for Painting {
    fn price(&self) -> f32 {
        return self.current_price;
    }

    fn description(&self) -> &str {
        return &self.title;
    }
}

pub enum BST {
    Leaf {
        value: Box<dyn Valueable>,
        left: Box<BST>,
        right: Box<BST>,
    },
    Empty,
}



impl BST {
    pub fn new() -> Self {
        BST::Empty
    }

    pub fn add(&mut self, new_value: Box<dyn Valueable>) {
        match self {
            BST::Leaf {
                value,
                left,
                right,
            } => match new_value.price().partial_cmp(&value.price() ) {
            Some(Ordering::Less) => left.add(new_value),
            Some(Ordering::Greater) => right.add(new_value),
            Some(Ordering::Equal) => return,
            None => todo!(),
        },
        BST::Empty => {
            *self = BST::Leaf {
                value: new_value,
                left: Box::new(BST::Empty),
                right: Box::new(BST::Empty),
            }
        }
    }
    }

    pub fn contains(&self, find_value: Box<dyn Valueable>) -> bool {
        match self {
            BST::Leaf {
                value,
                left,
                right,
            } => match find_value.price().total_cmp(&value.price()) {
                Ordering::Less => left.contains(find_value),
                Ordering::Greater => right.contains(find_value),
                Ordering::Equal => true,
            },
            BST::Empty => false,
        }
    }

    pub fn as_vector(&mut self) -> Vec::<&str> {
        match self {
            BST::Empty => return Vec::new(),
            BST::Leaf { value, left ,right } => {
                let mut values = right.as_vector();
                values.push(self.to_string());
                values.extend(left.as_vector());
                values
            },
        }
    }
}


fn main(){
    let mut bst = BST::new();

    let car1 = Car {
        brand: "Volvo".to_string(),
        model: "123ABC".to_string(),
        original_price: 23000.50,
        age: 12,
    };
    BST::add(&mut bst, Box::new(car1.clone()));

    let painting1 = Painting {
        artist: "Hans".to_string(),
        title: "Hanswacht".to_string(),
        current_price: 999.33,
    };

    BST::add(&mut bst, Box::new(painting1));

    BST::contains(&mut bst, Box::new(car1.clone()));
    BST::as_vector(&mut bst);
}

Compiler output

error[E0599]: the method `to_string` exists for mutable reference `&mut BST`, but its trait bounds were not satisfied
   --> main.rs:100:34
    |
 42 | pub enum BST {
    | ------------ doesn't satisfy `BST: ToString` or `BST: std::fmt::Display`
...
100 |                 values.push(self.to_string());
    |                                  ^^^^^^^^^ method cannot be called on `&mut BST` due to unsatisfied trait bounds
    |
    = note: the following trait bounds were not satisfied:
            `&mut BST: std::fmt::Display`
            which is required by `&mut BST: ToString`
            `BST: std::fmt::Display`
            which is required by `BST: ToString`
note: the trait `std::fmt::Display` must be implemented
   --> library/core/src/fmt/mod.rs:1187:0
    = help: items from traits can only be used if the trait is implemented and in scope
    = note: the following trait defines an item `to_string`, perhaps you need to implement it:
            candidate #1: `ToString`

For more information about this error, try `rustc --explain E0599`.
error: could not compile `src` (bin "src") due to 1 previous error

use .partial_cmp() instead, which is from PartialOrd. or alternatively, use an if-else chain with the comparison operators like <, ==, >.

Using the if-else chain seems easier to do, but I could not find online how to do so in a match.

total_cmp can handles negative values just fine and the same as regular comparison.
it's only NaNs which are different.

I utilized partial_cmp as nerditation suggested.

that would mean that a car and a painting of the same price would be considered equal, which is quite weird, but if that it what you want to do you can simply write find_value.price().total_cmp(&value.price()).

It is weird, but it was the assignment. It does work too, thank you!

Thank you both for the help I think I can continue now.

7

you don't do it "in" a match, you replace the match with a chain of if-else clauses.

for example, to compare two f32 values, the following two form have the same behavior:

// example values to compare
let price1 = 42f32;
let price2 = 1_000_000f32;

// using `match`
match price1.partial_cmp(&price2) {
    Some(Less) => println!("price 1 is less"),
    Some(Greater) => println!("price 1 is greater"),
    Some(Equal) => println!("prices are equal"),
    None => println!("at least one price is NaN"),
}

// using if-else chain
if price1 < price2 {
    println!("price 1 is less")
} else if price1 > price2 {
    println!("price 1 is greater") 
} else if price1 == price2 {
    println!("prices are equal")
} else {
    println!("at least one price is NaN")
}

it seems you are very early in the course, given you don't know how comparison works yet, which makes it even weirder for an assignment to use trait objects?

or, is it a rust course at all, or is it some OOP course with rust being your personal choice of language? I strongly suggest to NOT learn rust using OOP paradigms. but it's my personal opinion anyway.

8

you don't do it "in" a match, you replace the match with a chain of if-else clauses.

I see I misread your comment, sorry for that. Thank you for the example nonetheless.

it seems you are very early in the course, given you don't know how comparison works yet, which makes it even weirder for an assignment to use trait objects? or, is it a rust course at all, or is it some OOP course with rust being your personal choice of language? I strongly suggest to NOT learn rust using OOP paradigms. but it's my personal opinion anyway.

My Rust knowledge is not that great, since I sometimes find interpeting the Rust documentation and finding what I need difficult. This is one of the later assignments by the way. I have done a few other assignments which were easier. If it interests you this was the assignment:

Start in a new Rust file.

Now let's supposed a filthy rich client offers you a lot of money to create a digital catalogue for her most valuable possessions: sports cars and paintings. The catalogue should at any time be sorted by the value of each object, starting with the most expensive object (of course!). So let's use a binary search tree for that!

The problem is that we need to store different information for both types of valuables. Paintings have an artist, a title and the current price. Sports cars have a brand, a model, an original price and an age (in years). The current price of the car is calculated as original_price * (0.95 ^ age_in_years). (Floating point numbers have a powf method.) Implement these as structs.

Create a Valuable trait that provides methods for getting the current price of the valuable and for getting a description. Implement this trait for both of your structs.

Create a copy of your binary search tree, and modify it so that it stores Valuables, ordered by descending price. Within the main function, insert a few cars and a few paintings into the tree, and use as_vector (which should still return a Vec<String>, calling to_string on each Valuable) to print a list of descriptions and current prices, demonstrating that items are stored in the tree in the right order.

Hint: you'll need to use trait objects. As the size for a trait object cannot be known at compile-time (sports cars will take a bit more memory than paintings, because of an extra field), they can only be allocated on the heap.