Compare memberless records by value, others by members about fluentassertions HOT 11 OPEN

Hmm.. I would rather try to build a CompareBy... method for every case instead of using bool parameter which in certain cases are overlapping (example 2 and 3 above, which does the same)

from fluentassertions.

Hmm.. I would rather try to build a CompareBy... method for every case instead of using bool parameter which in certain cases are overlapping (example 2 and 3 above, which does the same)

Can you make a concrete suggestion?

Do you mean something like:

CompareAllRecordsByMembers();
CompareRecordsByMembersButWhenTheyDontHaveMembersCompareByValue();
CompareAllRecordsByValue();

?

from fluentassertions.

I must be missing something, but what is there to compare between member-less records?

from fluentassertions.

Hi @dennisdoomen

I must be missing something, but what is there to compare between member-less records?

Their type. That's why I'd like to have value semantic for them.
But I guess the interesting question is not how to compare them, but why would we have member-less records in a data-only object graph (no behavior)?.
The answer to that is: we use discriminated unions. So the first relevant information is in which case the discriminated union is. The second relevant information may be the data the case contains - but sometimes the case is all the data there is - hence I've got this object without any members.

Long version:

Let me start with a simplified example. This shows why we actually have member-less records in our object graph: it's about representing state/data via discriminated unions.

public record class FeatureLevel
{
    public abstract T Match<T>(
         Func<Basic, T> basic,
         Func<Professional, T> professional);

    public record class Basic : FeatureLevel
    {
         public override T Match<T>(
             Func<Basic, T> basic,
             Func<Professional, T> professional)
         {
             return basic(this);
         }
    }
    
    public record class Professional : FeatureLevel
    {
         public required int LicenseCount { get; init; }
        
         public override T Match<T>(
             Func<Basic, T> basic,
             Func<Professional, T> professional)
         {
             return professional(this);
         }
    }
}    

So usage would be:

FeatureLevel result = testee.ComputeFeatureLevel(...);

result.Should().BeEquivalentTo(
    new FeatureLevel.Professional
    {
        LicenseCount = 7
    });

which could result in an error similar to:

expected result to be
FeatureLevel.Professional { LicenseCount = 7 }
but found
FeatureLevel.Professional { LicenseCount = 3 }

or

expected result to be
FeatureLevel.Professional { LicenseCount = 7 }
but found
FeatureLevel.Basic

Now this example is not very interesting yet, because I could use BeEquivalentTo(...) when expecting a record with members (FeatureLevel.Professional). but just switch to Be(..) when expecting a member-less record FeatureLevel.Basic.

However, in real life we commonly have records which are more complex (= bigger tree of objects), so both, records with and without members, exist in the same tree. So this is what we do:

BeEquivalentTo(
    ....,
    options => options
         .CompareByValue<TypeWithoutMembers>()
         .CompareByValue<OtherTypeWithoutMembers>()
         .CompareByValue<YetAnotherTypeWithoutMembers>());

That's quite cumbersome and error prone. It's often a few rounds of trial and error until I've specified all the ones I need. And then, If ever I remove a without-members type property from a record, I should remove its .CompareByValue<..> as well. So it creates additional maintenance burden.
If we were able to configure assertions to configure records by members, but records without members by value - per default - we could basically get rid of all the CompareByValue<T> calls.

Sidenote: now that there is records, mostly the benefit of Should().BeEquivalent() for us is not that it provides equality comparison where there is no viable Equals() implementation, but that when inequality is detected, we get a nice message stating what exactly is inequal. That makes it valuable and preferable over Should().Be(..).
Actually, in the roughly 10 years I've been using FluentAssertions, the detailed message in BeEquivalent(..) telling me exactly what's different was and is the compelling feature. The second one would be collection comparison ;-)

So thank you @dennisdoomen and @jnyrup for this great library! And especially the continued maintenance and support of it :-)

from fluentassertions.

It's a bit of a niche feature, but I would be fine with somebody contributing an option like ComparingMemberlessRecordsByValue. What do you think @jnyrup ?

from fluentassertions.

This problem is not unique to records but memberless types in general, e.g. #2391.
The general question is how to compare memberless objects?

• Structural equivalency implies that it's equivalent to all other empty types. (Which is what we disallow currently)
• Value semantics rarely makes sense for types that doesn't override Equals.

I see how using value semantics on memberless records makes sense, since synthesized Equals will be a check if the types are exactly the same, which often seems what users want to compare.

I'm wondering if there is a more general applicable approach that could also work for types without overridden Equals.

from fluentassertions.

Maybe something like ComparingMemberlessTypesByValue?

from fluentassertions.

@dennisdoomen @jnyrup

So the options are as follows, correct?

• using expected.Equals(actual):
  • CompareMemberlessTypesByValue (on all memberless types)
  • CompareMemberlessRecordsByValue (on memberless records only)
• using actual.GetType() == expected.GetType()
  • CompareMemberlessTypesByType (on all memberelss type)
  • CompareMemberlessRecordsByType (on memberless records only)

Out of these I'm rather opinionated on CompareMemberlessTypesByValue: I think it's a bad option, because BeEquivalentTo() means we don't have to implement Equals() ourselves, and this would violate this principle.

Furthermore, I can make a small case as that it should only affect records:

Structural equivalency implies that it's equivalent to all other empty types. (Which is what we disallow currently)

This behavior helps preventing "false positive" tests in the following cases:
-a) the type does have meaning, so it's not just about structural equivalency
-b) I (accidentally?) didn't "publish" my members on the type (private)

For a) it doesn't matter whether the involved types are records or not.
I think though, that for b) it does: in the case of a records, it's very likely that it features a generated .Equals() implementation which would consider these private members. The opposite is true for non-record classes.

=> The likelihood of false positives with records is smaller.

Counterargument would be, that problem b) also occurs when part of your members are private, and some are not. So there's no reliable fix for it - so it could create more confusion than it's worth?

Anyway, I'm sure you guys have better insight into this, so in case you both can agree on a variant as acceptable for a PR, I'd happily do that one.

from fluentassertions.

I that case, CompareMemberlessTypesByType makes more sense to me, since that's what your intention is.

from fluentassertions.

Ok, so we'd have

• ComparingRecordsByValue()
• ComparingRecordsByMembers()
• CompareMemberlessTypesByType()

How should the implementation respect combinations of these? Should it (not) matter in which sequence these are called?
For example

.ComparingRecordsByMembers()
.CompareMemberlessTypesByType()

=> Memberless records compared by type
=> Records with members compared by members

vs

.CompareMemberlessTypesByType()
.ComparingRecordsByMembers()

=> Memberless non records compared by type, but
=> Memberless records compared by member?

or should memberless records still be compared by type?

from fluentassertions.

It doesn't matter. If the type is memberless, it's compared by type. But now I'm in doubt again whether CompareMemberlessTypesByValue isn't the better option. Also considering #1860.

from fluentassertions.