Fix bug in ValueObject.Equals

[rmeoc]

This fixes the issue described here: dotnet/docs#7477

The bug in ValueObject.Equals occurs when comparing two ValueObjects that have equal atomic values except that one of the two ValuesObjects has one atomic value more than the other. In that case ValueObject.Equals returned true even though the objects differ.

[eOkadas]

while(true) seems like a practice to avoid. Would I be wrong in suggesting the following ?

            while (thisValues.MoveNext() | otherValues.MoveNext())

[rmeoc]

@eOkadas, thanks for your suggestion. I don't think that will work though.

I have a feeling that perhaps you meant
while (thisValues.MoveNext() & otherValues.MoveNext())
because that would break the loop if either thisValues or otherValues reaches the end of the collection.

Using '&' or '|' instead of '&&' or '||' makes sure that MoveNext will be called on both thisValues and otherValues, keeping the enumerators in sync as it were, but it will not help in determining whether a single enumerator reached the end of the collection or both enumerators reached the end of the collection. If both thisValues and otherValues reach the end of the collection at the same time then the collections are equal but if one of them reaches the end before the other does then the collections are not equal because one collection has more elements than the other.

You can call MoveNext again but it could be that the previous call to MoveNext moved the enumerator to the last element. In that case the previous call returned true but calling it again will not return true anymore because MoveNext cannot move any further.

[eOkadas]

I thought | might work because because the issue at hand was having more atomic values on one side.
Calling moveNext until both collections really reach their end this way.
using & ensures both MoveNexts are evaluated but doesn't get us inside the while loop.

With | we evaluate both, get in the loop and if any of the two Current values is null and not the other we return false as we hope. The first if gets us out.

              if (ReferenceEquals(thisValues.Current, null) ^ ReferenceEquals(otherValues.Current, null))
                {
                    return false;
                }

[rmeoc]

You would then assume that neither collection contains any nulls. Suppose that both collections are equal except that one collection has one element more than the other and that the value of that additional element is null. With the solution you suggest these collections would be considered equal. When one of the iterators has reached the end, both return null when their current value is evaluated: one because it reached the end[1], the other because its current element is actually null.

[1] Actually the value of IEnumerator.Current is undefined if the last call to MoveNext returned false, see https://docs.microsoft.com/en-us/dotnet/api/system.collections.generic.ienumerator-1.current

[eOkadas]

Thanks I love that you took the time to clear that up.

[rmeoc]

I'm closing this pull request in favor of #1316 because I think that solution is better.