vkhorikov / CSharpFunctionalExtensions

Functional extensions for C#
MIT License
2.43k stars 303 forks source link

Making ValueObject.GetEqualityComponents return IComparable is a breaking change #509

Open robertlarkins opened 1 year ago

robertlarkins commented 1 year ago

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?

vkhorikov commented 1 year ago

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?

robertlarkins commented 1 year ago

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?

vkhorikov commented 1 year ago

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

robertlarkins commented 1 year ago

PR #510 submitted for getting Entity to implement IComparable.

vkhorikov commented 1 year ago

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

Szuuuken commented 1 year ago

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?

vkhorikov commented 1 year ago

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.

znamenap commented 7 months ago

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

My example TextValue implements IComparable. 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

vkhorikov commented 7 months ago

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