Replace ValueObject implementation

[KevinSchiener]

The current ValueObject implementation has three drawbacks:

1. It uses reflection to get the properties,
2. Unless you know the code it's hard to modify equality, e.g. if equality should be case-insensitive and
3. the declaration is somewhat awkward with with the generic parameter that has to be the same as the class.

The ValueObject proposed here or here (Microsoft) seems to solve all those issues and seems to me the most commonly used pattern in newer projects.

Example from Microsoft:

public abstract class ValueObject
{
    protected static bool EqualOperator(ValueObject left, ValueObject right)
    {
        if (ReferenceEquals(left, null) ^ ReferenceEquals(right, null))
        {
            return false;
        }
        return ReferenceEquals(left, null) || left.Equals(right);
    }

    protected static bool NotEqualOperator(ValueObject left, ValueObject right)
    {
        return !(EqualOperator(left, right));
    }

    protected abstract IEnumerable<object> GetAtomicValues();

    public override bool Equals(object obj)
    {
        if (obj == null || obj.GetType() != GetType())
        {
            return false;
        }

        ValueObject other = (ValueObject)obj;
        IEnumerator<object> thisValues = GetAtomicValues().GetEnumerator();
        IEnumerator<object> otherValues = other.GetAtomicValues().GetEnumerator();
        while (thisValues.MoveNext() && otherValues.MoveNext())
        {
            if (ReferenceEquals(thisValues.Current, null) ^
                ReferenceEquals(otherValues.Current, null))
            {
                return false;
            }

            if (thisValues.Current != null &&
                !thisValues.Current.Equals(otherValues.Current))
            {
                return false;
            }
        }
        return !thisValues.MoveNext() && !otherValues.MoveNext();
    }

    public override int GetHashCode()
    {
        return GetAtomicValues()
         .Select(x => x != null ? x.GetHashCode() : 0)
         .Aggregate((x, y) => x ^ y);
    }        
    // Other utilility methods
}

Usage:

public class Address : ValueObject
{
    public String Street { get; private set; }
    public String City { get; private set; }
    public String State { get; private set; }
    public String Country { get; private set; }
    public String ZipCode { get; private set; }

    private Address() { }

    public Address(string street, string city, string state, string country, string zipcode)
    {
        Street = street;
        City = city;
        State = state;
        Country = country;
        ZipCode = zipcode;
    }

    protected override IEnumerable<object> GetAtomicValues()
    {
        // Using a yield return statement to return each element one at a time
        yield return Street;
        yield return City;
        yield return State;
        yield return Country;
        yield return ZipCode;
    }
}

I'd suggest using this implementation instead.

[hitaspdotnet]

I used this instead of the ABP built-in ValueObject

[hikalkan]

Thank you for your explanation. It seems reasonable, I will change it :)

[viewtance]

@hikalkan Will you implement this enhancement for ABP too?

[hikalkan]

We can not replace the current one since it is a big breaking change, but we may have two base classes (one is existing generic, and other one is this new one). What do you think? I've just created an issue for it: aspnetboilerplate/aspnetboilerplate#4304

[gitlsl]

since this is a new project and has not release 1.0 , so we can use the new implement,short pain is better than long pain