Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
robertlarkins opened this issue Jun 24, 2023 · 9 comments

Comments

@robertlarkins
Copy link
Contributor

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
Copy link
Owner

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
Copy link
Contributor Author

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
Copy link
Owner

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

@robertlarkins
Copy link
Contributor Author

PR #510 submitted for getting Entity to implement IComparable.

@vkhorikov
Copy link
Owner

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

@Szuuuken
Copy link

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
Copy link
Owner

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
Copy link

znamenap commented Mar 12, 2024

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
Copy link
Owner

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants