CSharpFunctionalExtensions icon indicating copy to clipboard operation
CSharpFunctionalExtensions copied to clipboard

Making ValueObject.GetEqualityComponents return IComparable is a breaking change

Open robertlarkins opened this issue 2 years ago • 9 comments

Previously GetEqualityComponents() returned IEnumerable<object>. Now it returns IEnumerable<IComparable>. If the ValueObject holds an Entity and it is used as an Equality Component it causes a compile time error if the Entity doesn't implement IComparable.

What is the appropriate fix here? Every type that ValueObject uses as an Equality Component must now implement IComparable? Could this extend to the CSharpFunctionalExtensions Entity type so entities have it is available out-of-the-box?

robertlarkins avatar Jun 24 '23 11:06 robertlarkins

What is the appropriate fix here? Every type that ValueObject uses as an Equality Component must now implement IComparable?

Yes, unfortunately, that's the only way. Otherwise, the comparisons can be inconsistent.

Could this extend to the CSharpFunctionalExtensions Entity type so entities have it is available out-of-the-box?

You mean make Entity implement IComparable?

vkhorikov avatar Jun 27 '23 12:06 vkhorikov

You mean make Entity implement IComparable?

Yes, then an Entity could be used in a ValueObject as an equality component without needing to explicitly implement IComparable on the derived entity type. Id would be the property used for the comparison.

Or is there issues I'm not seeing with Entity implementing ICompariable?

robertlarkins avatar Jun 27 '23 19:06 robertlarkins

Yes, that's a good idea. No particular reason why it's not done yet. Appreciate if you could submit a PR.

vkhorikov avatar Jun 27 '23 22:06 vkhorikov

PR #510 submitted for getting Entity to implement IComparable.

robertlarkins avatar Jun 28 '23 09:06 robertlarkins

Thank you. I'll double check everything this weekend and release a new minor version then.

vkhorikov avatar Jun 28 '23 13:06 vkhorikov

Hi, this is also a breaking change for any kind of list or array ValueObjects. The following Examples aren't possible any more:

Value Object as an byte array
public class FileContent : SimpleValueObject<byte[]>
{
    public FileContent(byte[] value) : base(value)
    {
    }
}

Value Object as a List of ValueObjects
public class Thing: SimpleValueObject<string>
{
    public Thing(string value) : base(value)
    {
    }
}

public class MultpleThings: SimpleValueObject<List<Thing>>
{
    public MultpleThings(List<Thing> value) : base(value)
    {
    }
}

Do you have any suggestions how to solve such scenarios?

Szuuuken avatar Aug 22 '23 08:08 Szuuuken

You'll need to fall back to the base ValueObject:

public class FileContent : ValueObject
{
    protected override IEnumerable<IComparable> GetEqualityComponents()
    {
        yield return Value.GetHashCode();
    }
}

This would make so you can't really compare two instances of FileContent, but you couldn't do that in the previous version either.

Alternatively, if you really want to make this comparison, you can do this:

public class FileContent : ValueObject
{
    protected override IEnumerable<IComparable> GetEqualityComponents()
    {
        foreach (byte b in Value)
        {
            yield return b;
        }
    }
}

But that would most likely make it impractical performance-wise.

Same for MultpleThings.

vkhorikov avatar Aug 24 '23 17:08 vkhorikov

Hi, Are there any plans to give Maybe<T> the ability to honor IComparable of the T? image

My example TextValue implements IComparable<TextValue>. Nevertheless, due to the breaking change, this fails to compile.

I'll probably convert it into this form: image

Thanks for any opinion or advice. Pavel

znamenap avatar Mar 12 '24 22:03 znamenap

Yeah, we should do that too. Feel free to submit a PR.

vkhorikov avatar Mar 15 '24 09:03 vkhorikov