Best practice PartialEq

Hello

I'm writing a library to deparse GDTF Global Device Type Format in entertainment industry files.

The Format is a tree of a lot of structs. On some structs I want to implement PartialEq for anyone to uses the library to be able to compare some structs. In a lot of these cases it would not be intuitive if they match in any case, even when the data structure is the same.

Example
The struct DataVersion contains the GDTF Version the file was created with. I want to be able to compare this versions but I don't want to return true if the Version is not known / not yet supported by the library

assert_eq!(DataVersion::Version1_0, DataVersion::Version1_0);
assert_ne!(DataVersion::Unknown, DataVersion::Unknown);

So far so good but I want to write test to be able to compare DataVersion::Unknown. I've created a new trait AssertEqAllowEmpty for that reason.
To keep code slim and not to confuse anyone using this library I've added #[cfg(test)] to this trait.

When starting to get better documentation on every method, the common way is to use assert_eq!. You are able to test this code also to be sure your doc is correct what is great!

But when I now would write somthing like

assert_ne!(DataVersion::Unknown, DataVersion::new_from_str("1.something invalid"));

in the documentation, I would be really confused as user because they are the same in that context.

Also I can't document it like

assert!(DataVersion::Unknown.is_eq_allow_empty(&DataVersion::new_from_str("1.something invalid"));

because this method exists only in testing context.

What would you suggest is best practice to solve all this problems?

If you need context aware comparison, I think it would be better to use aptly named traits. However, you can always document your example code to clarify that PartialEq for this type has some specific meaning and point them to the docs about it.

After all, Ruby get's away with having ==, ===, eq? and equal?. And no, it's not like == == eq? and === == equal? either, all 4 of them mean something different.

1 Like

Personally, I would provide a public-facing eq_exact method (or something similar) that you can use to write tests and demonstrate exact equality. You may thinl such things are only useful for testing, but anybody using your library might want to ensure their own tests are capable of making the same assertions on their own outputs. There's little reason to hide this functionality behind cfg[test] if it is useful and well-documented.

Addendum: When trying to think of what the semantics of the actual == operator should be, it might make the most sense to think about what your users would expect when your type is used as a key in a BTreeMap or HashMap, etc., because those are probably the most common uses of generic Eq bounds.

2 Likes

Would you be open to a structure such as

#[derive(PartialEq, Debug)]
enum DataVersion {
    Version1_0,
    Unknown(String),
}

#[test]
fn test_eq() {
    assert_eq!(DataVersion::Version1_0, DataVersion::Version1_0);
    assert_eq!(DataVersion::Unknown("1.foo".into()), DataVersion::Unknown("1.foo".into()));
    assert_ne!(DataVersion::Unknown("1.foo".into()), DataVersion::Unknown("1.bar".into()));
}

? That is, rather than a blanket unknown, acknowledging the existence of nonstandard variant versions at the type level, and allowing them to compare equal to one another?

You can extend this to one which allows both variants and unknown versions, as well:

#[derive(Debug)]
enum DataVersion {
    Version1_0,
    Variant(String),
    Unknown,
}

impl PartialEq for DataVersion {
    fn eq(&self, other: &Self) -> bool {
        use DataVersion::*;
    
        match (self, other) {
            (Version1_0,     Version1_0)     => true,
            (Unknown,        Unknown)        => false,
            (Variant(ref a), Variant(ref b)) => *a == *b,
            (_,              _)              => false,
        }
    }
}

#[test]
fn test_eq() {
    assert_eq!(DataVersion::Version1_0, DataVersion::Version1_0);
    assert_eq!(DataVersion::Variant("1.foo".into()), DataVersion::Variant("1.foo".into()));
    assert_ne!(DataVersion::Variant("1.foo".into()), DataVersion::Variant("1.bar".into()));
    assert_ne!(DataVersion::Unknown, DataVersion::Unknown);
}

The other direction I'd look is into whether two unknown versions really are incomparable with one another like this. I think I see why you want to model it this way, but there's a reason NaN is often a challenge to handle right.

Edit: a third option would be to remove the Unknown variant entirely, implement Eq as well as PartialEq, and use Option<DataVersion> in contexts where a version might not be known. That forces more care in those contexts, while still allowing a fairly natural use of == to compare Nones.

1 Like

The BTreeMap and HashMap thinking what lead to my new trait in the first place.

Thanks for the advice!

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.