pretzelhammer / rust-blog

Educational blog posts for Rust beginners
Apache License 2.0
7.53k stars 396 forks source link

PartialEq example isn't right #30

Closed ChrisJefferson closed 3 years ago

ChrisJefferson commented 3 years ago

The "Kelvin / Celcius / Fahrenheit" example for PartialEq isn't right, as floating point numbers suck.

I expected to break transitivity, but it turns out even symmetry is broken fairly easily, see for example:

https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=754a08485eb182e995f3e9889243dd81

To fix, while much less fun, maybe do millimeters, centimeters, kilometers? If they are each stored as i32, you could make the millimeters in i64 for comparison, which deals with overflow.

pretzelhammer commented 3 years ago

Oof, yeah, I was worried about this, I should have stress-tested that example more. I switched it to distances, this is the example in the blog post now:

#[derive(PartialEq)]
struct Foot(u32);

#[derive(PartialEq)]
struct Yard(u32);

#[derive(PartialEq)]
struct Mile(u32);

impl PartialEq<Mile> for Foot {
    fn eq(&self, other: &Mile) -> bool {
        self.0 == other.0 * 5280
    }
}

impl PartialEq<Foot> for Mile {
    fn eq(&self, other: &Foot) -> bool {
        self.0 * 5280 == other.0
    }    
}

impl PartialEq<Mile> for Yard {
    fn eq(&self, other: &Mile) -> bool {
        self.0 == other.0 * 1760
    }
}

impl PartialEq<Yard> for Mile {
    fn eq(&self, other: &Yard) -> bool {
        self.0 * 1760 == other.0
    }    
}

impl PartialEq<Foot> for Yard {
    fn eq(&self, other: &Foot) -> bool {
        self.0 * 3 == other.0
    }
}

impl PartialEq<Yard> for Foot {
    fn eq(&self, other: &Yard) -> bool {
        self.0 == other.0 * 3
    }
}

fn main() {
    let a = Foot(5280);
    let b = Yard(1760);
    let c = Mile(1);

    // symmetry
    assert!(a == b && b == a); // ✅
    assert!(b == c && c == b); // ✅
    assert!(a == c && c == a); // ✅

    // transitivity
    assert!(a == b && b == c && a == c); // ✅
    assert!(c == b && b == a && c == a); // ✅
}

This is probably better than the temperature example, but it's still unideal because if someone was realistically writing this program they would use f64s and not u32s. I'm open to suggestions on how maybe this example can be improved further, or replaced for something better, so I'll leave this issue open for now.

ChrisJefferson commented 3 years ago

I think this is better, I'll be honest, I was just curious if it did work, seeing floating points and == always sets of my "smell test", so the important part is seeing what it looks like generally.

Technically, this can overflow, but I'm not sure it worth fixing that, for a simple example -- 2^32 inches will cover any distance on Earth, and people shouldn't be measuring inter-stellar distances in inches ;)