SteveDunn / Vogen

A semi-opinionated library which is a source generator and a code analyser. It Source generates Value Objects
Apache License 2.0
781 stars 45 forks source link

Record ValueObject does not have equality check for type vs underlying type #256

Closed timbze closed 1 year ago

timbze commented 1 year ago

I guess (or hope) you don't need all the equality overrides as record already does it oob.

Originally posted by @MortenMeisler in https://github.com/SteveDunn/Vogen/issues/128#issuecomment-1116957865

I saw that comment. It seems records do not support equality check between the record and it's underlying type. E.g. For this type:

[ValueObject(typeof(int), conversions: Conversions.LinqToDbValueConverter)]
public partial record TenantId

I get this error when trying to compare TenantId to int

Cannot apply operator '==' to operands of type 'Infra.Database.Common.TenantId' and 'int', candidates are:
bool ==(Infra.Database.Common.TenantId?, Infra.Database.Common.TenantId?) (in record TenantId)
timbze commented 1 year ago

Since I got #255 figured out this is not important to me for now... but is maybe a good feature request?

SteveDunn commented 1 year ago

Thanks for raising this. It's an interesting question. Is a TenantId with a value a of 123 equal to an integer with a value of 123? If yes, then the reverse must also be true, an integer with a value of 123 is equal to a TentantId with a valid of 123.

Taken to its logical conclusion, if Integer(123) == TenantId(123), then we could remove the type and just say that 123 == 123, and hence shift-delete Vogen! :)

It is a good question though. It relates to another feature request that I've seen (and at one point, wanted (and even implmented)), and that is of implicit conversions:

    public static implicit operator int(TenantId d) => d.Value;
    public static implicit operator TenantId(int b) => TenantId.From(b);

There's three problems with this:

  1. it really does say that a TenantId is an integer
  2. it might throw an exception - which it shouldn't
  3. it removes most of the benefits of having value objects, because the C# compiler will happily let you interchange any value object with any other value object

On that last point, if we have a method like: public void Process(CustomerId c, AccountId a)

we can now do

int customerId = 123;
int accountId = 321;
Process(accountId, customerId); // <-- wrong way round

... and even worse!

CustomerId customerId = CustomerId.From(123);
AccountId accountId = AccountId.From(321);
Process(accountId, customerId); // <-- wrong way round

The latter works because there is an implict conversion to int from the value objects, and an implicit conversion from int to the value ojbects.

I'll close this for now, but if you feel strongly about this, then we can chat further (it might be something that could be opted-in to when users know the consequences)