Feature request: add enum flag to skip producing IsInitialized comparison within Equals method

[amyboose]

Describe the feature

First, I want to thanks for good project. But I have some problems to make it works for my project.

In current version Vogen generate next Equals method:

public readonly global::System.Boolean Equals(UserId other)
{
    // It's possible to create uninitialized instances via converters such as EfCore (HasDefaultValue), which call Equals.
    // We treat anything uninitialized as not equal to anything, even other uninitialized instances of this type.
    if (!IsInitialized() || !other.IsInitialized())
        return false;
    return global::System.Collections.Generic.EqualityComparer<System.Int32>.Default.Equals(Value, other.Value);
}

But I want to have only simple equlity comparer:

public readonly global::System.Boolean Equals(UserId other)
{
    return global::System.Collections.Generic.EqualityComparer<System.Int32>.Default.Equals(Value, other.Value);
}

Often I should compare my value with default value and it is simple for many programmers to understand default comparison way.
For example:

//Some code to get UserId. It can be request to database
UserId userId = UserId.From(0);
//Check if default:
#pragma warning disable VOG009 // Using default of Value Objects is prohibited
if (userId is default) //false
{
    //do something 
}

It looks simple and many developers can understand this. Yes, It good, that there is method IsInitialized, but in real world scenarios some programmers quit and others are hired. And when the program is simple, then it is better.
The other problem is that last comparison with default returns false. And it is so unobvious. I prefer to avoid such situations.

And I have one more problem with my transition from StronglyTypedId. Vogen behaviour doesnt work for me. The reason is that I use my own EF ValueConverter and it looks so:

internal sealed class UserIdConverter : ValueConverter<UserId, int>
{
    public UserIdConverter()
        : base(x => x.Value, CreatePrimitiveExpression())
    {

    }

    private static Expression<Func<int, UserId>> CreatePrimitiveExpression()
    {
        // Generate expression
        // x => new UserId(x);
        // it works good even with private constructor
        // omit to simplified
    }
}

My value converter factory looks different but I conveyed the main meaning.

I can write separate post why I use private constructor instead of From and similar methods. But in current context it is not so important. What matters is that my ValueConverter and entity creation doesn't work together within Vogen. It works well with StronglyTypedId. It is because I don't use ValueComparer.
I know how Vogen EF Comparer works:

static bool DoCompare(RN.BNIPI.Domain.Common.Models.UserId left, RN.BNIPI.Domain.Common.Models.UserId right)
{
    // if neither are initialized, then they're equal
    if (!left.IsInitialized() && !right.IsInitialized()) return true;

    return left.IsInitialized() && right.IsInitialized() && UnderlyingValue(left).Equals(UnderlyingValue(right));
}

But it looks like an overengineering only to make it works. I think that own comparison methods of ValueObjects must be enough. But, as I mentioned early, comparison to default using Vogen returns false and it is the problem.

[SteveDunn]

Hi @amyboose - thank you for the feedback.

Re. Equals: it must first check if the left or right is initialised, otherwise an exception is thrown when Value is accessed on an unitialised instance.

Re. comparing to default: I think default is more of an operation on a primitive, for example, the default int is 0, but what is the default NationalInsuranceNumber (which wraps an int). And also, if the value object is a class, then default is null.

There is a way to remove validation though, which might help you achieve what you need.
With this set, Vogen produces this:

#if VOGEN_NO_VALIDATION
  public readonly bool IsInitialized() => true;
#else
    public readonly bool IsInitialized() => _isInitialized;
#endif

The Equals method remains unchanged but because IsInitialized is now always true, it now just compares the primitive. And with this flag set, Value (and EnsureInitialized) do not throw, they just return the default for that type (the default for a struct/value type, or null for a class/reference type)

I hope that helps. Thanks again for the feedback.

[amyboose]

Re. Equals: it must first check if the left or right is initialised, otherwise an exception is thrown when Value is accessed on an unitialised instance.

I think that the process of getting the value of a variable and the process of comparing values ​​may have different behavior. I agree, that for the process of obtaining the value of a variable it must generate an error. And I find IsInitialized() still very useful and don't want to get rid of it.

Re. comparing to default: I think default is more of an operation on a primitive, for example, the default int is 0, but what is the default NationalInsuranceNumber (which wraps an int). And also, if the value object is a class, then default is null.

It depends. For NationalInsuranceNumber probably it must be prohibit to compare it with C# default.

But in other cases it is useful compare to default. UserId, for example. Many developers who use EF Core can understand that default value is 0. And I find it good to have flexibility. But I can it disable and enable myself by using editorconfig and by other ways. And for these cases I want to use default.

There is a way to remove validation though, which might help you achieve what you need. With this set, Vogen produces this:

#if VOGEN_NO_VALIDATION
  public readonly bool IsInitialized() => true;
#else
    public readonly bool IsInitialized() => _isInitialized;
#endif

The Equals method remains unchanged but because IsInitialized is now always true, it now just compares the primitive. And with this flag set, Value (and EnsureInitialized) do not throw, they just return the default for that type (the default for a struct/value type, or null for a class/reference type)

As I mentioned before, I find IsInitialized() very useful. And, unfortunately, enabling VOGEN_NO_VALIDATION is very dangerous. It disables IsInitialized() check and allow to generate errors by writing code. And that's why I want to skip IsInitialized() check only in Equals method

[SteveDunn]

Hi @amyboose

It depends. For NationalInsuranceNumber probably it must be prohibit to compare it with C# default. But in other cases it is useful compare to default

I'll take a look this. My initial thoughts are that for classes, the C# compiler will just treat it as null, and for structs, it'll treat it as the default for the struct type; I also don't think it'll consider any overloaded == operators as, I think, default (like null) is typeless.