Comparing objects with a tolerance

Question

I have N child classes of the same base class. The base contains abstract methods to convert the class to a byte array and vice versa.

It also contains a method to compare that instance to two other instances of the same type. Result shall be true if this instance is within the range of the other two instances (each property value is within the range of the corresponding property values).

Collections shall be compared by comparing each value.

Because there are N childs I really don't want to implement the compare method in each child (huge amount of testing and boring code) so I use a property bag in the base class and implement the compare method there.

Child property implementation:

    public int IntValue
    {
        get { return m_IntValue; }
        set
        {
            m_IntValue = value;
            AddOrUpdateProperty(value);
        }
    }
    public string StringValue
    {
        get { return m_StringValue; }
        set
        {
            m_StringValue = value;
            AddOrUpdateProperty((IComparable)value);
        }
    }
    public byte[] ByteArray
    {
        get { return m_ByteArray; }
        set
        {
            m_ByteArray = value;
            AddOrUpdateProperty(value);
        }
    }

Methods to add or update the property bag Dictionary<string, object> m_PropertyBag. Only properties that implement IComparable are allowed.

private Dictionary<string, object> m_PropertyBag = new Dictionary<string, object>();

protected void AddOrUpdateProperty(IComparable value, [CallerMemberName]string name = null)
{
    AddOrUpdateProperty(value, name);
}

protected void AddOrUpdateProperty<T>(IEnumerable<T> value, [CallerMemberName]string name = null)
    where T : IComparable
{
    AddOrUpdateProperty(value, name);
}

private void AddOrUpdateProperty(object value, string name)
{
    if (value == null) { throw new ArgumentNullException("value"); }
    if (name == null) { throw new ArgumentNullException("name"); }
    if (name == string.Empty) { throw new ArgumentException("Name must not be empty.", "name"); }
    if (name.Contains(" ")) { throw new ArgumentException("Name must not contain any white spaces.", "name"); }

    if (m_PropertyBag.ContainsKey(name)) { m_PropertyBag[name] = value; }
    else { m_PropertyBag.Add(name, value); }
}

Here is the compare method for the whole bag.

public bool Compare(Base min, Base max)
{
    if (GetType() != min.GetType() || GetType() != max.GetType())
            throw new InvalidOperationException("Not the same type.");

    var result = true;
    foreach (var property in m_PropertyBag)
    {
        if (property.Value is IEnumerable)
        {
            var values = (IEnumerable)property.Value;
            var minValuesEnum = ((IEnumerable)min.m_PropertyBag[property.Key]).GetEnumerator();
            var maxValuesEnum = ((IEnumerable)max.m_PropertyBag[property.Key]).GetEnumerator();

            foreach (var value in values)
            {
                result &= minValuesEnum.MoveNext();
                result &= maxValuesEnum.MoveNext();
                if (!result) return false;

                result &= Compare(value, minValuesEnum.Current, maxValuesEnum.Current);
                if (!result) return false;
            }
        }
        else
        {
            result &= Compare(property.Value, min.m_PropertyBag[property.Key], max.m_PropertyBag[property.Key]);
            if (!result) return false;
        }
    }
    return result;
}

Here is the compare method for a single IComparable property.

public bool Compare(object value, object min, object max)
{
    var comparableObject = (IComparable)value;
    return comparableObject.CompareTo(min) >= 0 && comparableObject.CompareTo(min) <= 0;
}

Answer 1

It is a good idea to use consistent styles throughout your code. Right here, you use braces around a 1-line if/else block:

if (m_PropertyBag.ContainsKey(name)) { m_PropertyBag[name] = value; }
else { m_PropertyBag.Add(name, value); }

Right here, you do not:

if (GetType() != min.GetType() || GetType() != max.GetType())
        throw new InvalidOperationException("Not the same type.");

I recently switched to always using braces, but whatever you decide, be consistent.

---

I don't think you need all those assignments to result when you are just going to return if the value is false:

var result = true;
foreach (var property in m_PropertyBag)
{
    if (property.Value is IEnumerable)
    {
        var values = (IEnumerable)property.Value;
        var minValuesEnum = ((IEnumerable)min.m_PropertyBag[property.Key]).GetEnumerator();
        var maxValuesEnum = ((IEnumerable)max.m_PropertyBag[property.Key]).GetEnumerator();

        foreach (var value in values)
        {
            result &= minValuesEnum.MoveNext();
            result &= maxValuesEnum.MoveNext();
            if (!result) return false;

            result &= Compare(value, minValuesEnum.Current, maxValuesEnum.Current);
            if (!result) return false;
        }
    }
    else
    {
        result &= Compare(property.Value, min.m_PropertyBag[property.Key], max.m_PropertyBag[property.Key]);
        if (!result) return false;
    }
}
return result;

You can do this instead:

foreach (var property in m_PropertyBag)
{
    if (property.Value is IEnumerable)
    {
        var values = (IEnumerable)property.Value;
        var minValuesEnum = ((IEnumerable)min.m_PropertyBag[property.Key]).GetEnumerator();
        var maxValuesEnum = ((IEnumerable)max.m_PropertyBag[property.Key]).GetEnumerator();

        foreach (var value in values)
        {
            if (!minValuesEnum.MoveNext() ||
                !maxValuesEnum.MoveNext() ||
                !Compare(value, minValuesEnum.Current, maxValuesEnum.Current);
            {
                return false;
            }
        }
    }
    else if (!Compare(property.Value, min.m_PropertyBag[property.Key], max.m_PropertyBag[property.Key]))
    {
        return false;
    }
}
return true;

I'm not sure what is going to, or should, happen if neither the if nor the else if statements execute. Should there be an else { return false; } statement in there?