Skip to content

Possible issues with the IComparable interface and associated operators #1367

Closed
@lipchev

Description

@lipchev

Hey guys,
I've mentioned this issue back in #838 where the current implementation of the CompareTo is prone to the same rounding issues as with the old equality contract. Here's the test that I expect would still fail in the current code base:

    [Fact]
    public void CompareTo_WithSameQuantityInAnotherUnit_ReturnsZero()
    {
        var firstMass = Mass.FromGrams(0.001);
        var secondMass = firstMass.ToUnit(MassUnit.Microgram);

        Assert.Equal(0, firstMass.CompareTo(secondMass));
        Assert.Equal(0, secondMass.CompareTo(firstMass));
    }

This might not be an issue when sorting a collection (although it might not be obvious why sorting [x1, x2, x3] might yield a different ordering as compared to the result of sorting [x2, x1, x3]), however I'm quite worried about the > operator.

Here's the typical use-case that may be problematic:
if (mass > Capacity) // possible rounding error here

I'm starting think that my proposal for handling the result of both x1.As(x2.Unit).CompareTo(x2.Value) and x2.As(x1.Unit).CompareTo(x1.Value) might be "better" than what we have right now (covering 99% of the use cases) however, there is still the possibility of getting an unexpected result from Capacity.ToUnit(x).ToUnit(y) > Capacity..

I don't know, the stricter alternative is simply too painful to imagine..

PS There is still the option of trying to use rational numbers for the underlying type: now that there is only one interface to implement this could probably be hidden from the client (say we store the fraction inside the QuantityValue). There is probably just the issue with serializing a rational using the single-value contract (some precision maybe lost), however we could roll out a RationSerializationSerializer for those that need it..
Obviously this wouldn't be as "lightweight" as what we have right now, and there is also the possible issues with the external dependency (e.g. compatibility with the older/compact frameworks etc)..

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions