Implementing class equality/comparison in F#

Question

I'm working on some book material on classes and interfaces in F#. My example is a latitude/longitude class:

    open System

    type LatLon(latitude : float, longitude : float) =

        member __.Latitude = latitude
        member __.Longitude = longitude

        override this.GetHashCode() =
            hash (this.Latitude, this.Longitude)

        override this.Equals(thatObj) =
            match thatObj with
            | :? LatLon as that -> 
                this.Latitude = that.Latitude
                && this.Longitude = that.Longitude    
            | _ -> false

        interface IEquatable<LatLon> with
            member this.Equals(that : LatLon) =
                this.Equals(that)   

        interface IComparable with
            member this.CompareTo(thatObj) =
                match thatObj with
                | :? LatLon as that -> 
                    compare (this.Latitude, this.Longitude) (that.Latitude, that.Longitude)                    
                | _ -> 
                    raise <| ArgumentException("Can't compare instances of different types")

    let landsEnd = LatLon(50.07, -5.72)
    let johnOGroats = LatLon(58.64, -3.07)
    let landsEnd2 = LatLon(50.07, -5.72)

    let demo() =
        // false
        printfn "%b" (landsEnd = johnOGroats)
        // true
        printfn "%b" (landsEnd = landsEnd2)

        // 50.070000, -5.720000
        // 58.640000, -3.070000
        [ landsEnd; johnOGroats; landsEnd2 ] 
        |> Set.ofList
        |> Seq.iter (fun ll -> printfn "%f, %f" ll.Latitude ll.Longitude)

By the way, I'm well aware that in this case, a record type would give the necessary structural equality out of the box. But the material I want to cover with this example is specifically about classes.

My main concerns are:

• The code works the same if I don't implement IEquatable, as it uses the Equals() override. Is it considered desirable to implement IEquatable anyway, and/or does it have advantages I haven't spotted?

• What about the (currently very-lightly-documented) op_Equality? Should that be used as well/instead?

• The code works fine (for example Set construction, requiring comparison) if I implement IComparable but not IComparable<'T>. Have I missed out on anything useful by not using IComparable<'T>?

Obviously I'm amenable to any other feedback on the quality/style of this code.

Many thanks!

Answer 1

There's a suggestion for [<AllowNullLiteral>]: I think this is a bad idea.

You wrote a class that is a lat/long pair, as a result, it's a standard GeoCoordinate, which is a regular value-type, and should be marked as such. I get that the book calls it a class, but as an F# developer, how many times in your life have you actually had to think about the difference?

As a result, I'd add the structure definition in some manner, either via struct/end, or [<StructAttribute>].

I also recommend implementing the generic IComparable<T>. It's cheap, and can offer more performance to avoid boxing/unboxing should the caller need it. (Reed also covered this.) Some of the newer API's will try to rely on both, preferring the IComparable<T>. You may also want to read this answer to another question, as it talks about a lot of performance intricacies with regard to the standard BCL stuff. (You may want to create an IEqualityComparer<T>, for example.)

Some might argue that the guidelines say to keep the instance size "under 16 bytes", I personally think that's rubbish, as I'm pretty sure that should read "at or under 16 bytes". These are not always law.

The .NET BCL contradicts itself here several times (Size structure, Guid structure, too lazy to find the others), and the Struct Design guidelines don't mention it, only the class vs. struct guidelines.

As a result, I think you're safe in that regard here. The item you're talking about is small (16-bytes), represents a value (physical location that doesn't change), and should not be directly changed. Just as well, the copying performance can often use native SIMD instructions, so it should not be a big deal to copy 16-bytes. (I had a source on this, but I lost it, going to try to dig it up again.)

Answer 2

First off, given that this is about using classes, any real world scenario is likely going to do this because of interop with other languages (C#/VB/etc). Given that, I'd recommend adding [<AllowNullLiteral>], since usage from C# will likely result in null values polluting the usage of this type. You'd then need to write defensively around that.

The code works the same if I don't implement IEquatable, as it uses the Equals() override. Is it considered desirable to implement IEquatable anyway, and/or does it have advantages I haven't spotted?

Yes, it can be advantageous to do this, as it (can) avoid the boxing if written properly. In this case, you're delegating to your Equals method, so you lose that advantage.

If you make an Equals method that doesn't box, and have both implementations delegate to it, you gain back that advantage in many scenarios.

Normally, this is simple in C# - you'd use the implicit implementation of the interface for the main method, then override Object.Equals to call it. In F#, this is slightly less simple. I typically end up writing something like:

[<AllowNullLiteral>] // Note this added, since it'll come this way from C#/VB anyways
type LatLon(latitude : float, longitude : float) =
    let eq (that : LatLon) = 
        if isNull that then
            false
        else
            latitude = that.Latitude && longitude = that.Longitude        
// ... other code unchanged
    override this.Equals(thatObj) = 
        match thatObj with
        | :? LatLon as that -> eq that
        | _ -> false

    interface IEquatable<LatLon> with
        member this.Equals(that : LatLon) = eq that

What about the (currently very-lightly-documented) op_Equality? Should that be used as well/instead?

This can be useful for interop from C# or VB, as it will allow == to work from C#, etc.

The code works fine (for example Set construction, requiring comparison) if I implement IComparable but not IComparable<'T>. Have I missed out on anything useful by not using IComparable<'T>?

Mainly performance issues due to unnecessary boxing which will occur in many scenarios.

Answer 3

Don’t compare floats!

In Praxis there are only a few cases where spatial coordinates can be compared directly. In my 3D geometry code I even have an explicit NoComparison attribute on all my classes for points or vectors. Comparing floats should always be done with a tolerance value that is appropriate for your current domain. In case of Longitude and Latitude that tolerance would probably be one meter, or much more if you work with poor GPS signals.

For a book I would pick an example based on integers or strings. Where you don’t have to deal with all the problems of floats.

See http://floating-point-gui.de and https://docs.oracle.com/cd/E19957-01/806-3568/ncg_goldberg.html