HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Comparing objects with a tolerance

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
withtolerancecomparingobjects

Problem

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 m_PropertyBag. Only properties that implement IComparable are allowed.

```
private Dictionary m_PropertyBag = new Dictionary();

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

protected void AddOrUpdateProperty(IEnumerable 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 ArgumentExcepti

Solution

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?

Code Snippets

if (m_PropertyBag.ContainsKey(name)) { m_PropertyBag[name] = value; }
else { m_PropertyBag.Add(name, value); }
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;
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;

Context

StackExchange Code Review Q#90963, answer score: 4

Revisions (0)

No revisions yet.