A doubt about Dictionary<T,T>

Question

I have a class which looks like this:

public class NumericalRange:IEquatable<NumericalRange>
    {
        public double LowerLimit;
        public double UpperLimit;

        public NumericalRange(double lower, double upper)
        {
            LowerLimit = lower;
            UpperLimit = upper;
        }

        public bool DoesLieInRange(double n)
        {
            if (LowerLimit <= n && n <= UpperLimit)
                return true;
            else
                return false;
        }

        #region IEquatable<NumericalRange> Members

        public bool Equals(NumericalRange other)
        {
            if (Double.IsNaN(this.LowerLimit)&& Double.IsNaN(other.LowerLimit))
            {
                if (Double.IsNaN(this.UpperLimit)  && Double.IsNaN(other.UpperLimit))
                {
                    return true;
                }
            }

            if (this.LowerLimit == other.LowerLimit && this.UpperLimit == other.UpperLimit)
                return true;
            return false;
        }

        #endregion
    }

This class holds a neumerical range of values. This class should also be able to hold a default range, where both LowerLimit and UpperLimit are equal to Double.NaN.

Now this class goes into a Dictionary

The Dictionary works fine for 'non-NaN' numerical range values, but when the Key is {NaN,NaN} NumericalRange Object, then the dictionary throws a KeyNotFoundException.

What am I doing wrong? Is there any other interface that I have to implement?

Answer 1

Based on your comment, you haven't implemented GetHashCode. I'm amazed that the class works at all in a dictionary, unless you're always requesting the identical key that you put in. I would suggest an implementation of something like:

public override int GetHashCode()
{
    int hash = 17;
    hash = hash * 23 + UpperLimit.GetHashCode();
    hash = hash * 23 + LowerLimit.GetHashCode();
    return hash;
}

That assumes Double.GetHashCode() gives a consistent value for NaN. There are many values of NaN of course, and you may want to special case it to make sure they all give the same hash.

You should also override the Equals method inherited from Object:

public override bool Equals(Object other)
{
     return other != null && 
            other.GetType() == GetType() &&
            Equals((NumericalRange) other);
}

Note that the type check can be made more efficient by using as if you seal your class. Otherwise you'll get interesting asymmetries between x.Equals(y) and y.Equals(x) if someone derives another class from yours. Equality becomes tricky with inheritance.

You should also make your fields private, exposing them only as propertes. If this is going to be used as a key in a dictionary, I strongly recommend that you make them readonly, too. Changing the contents of a key when it's used in a dictionary is likely to lead to it being "unfindable" later.

Answer 2

The default implementation of the GetHashCode method uses the reference of the object rather than the values in the object. You have to use the same instance of the object as you used to put the data in the dictionary for that to work.

An implementation of GetHashCode that works simply creates a code from the hash codes of it's data members:

public int GetHashCode() {
   return LowerLimit.GetHashCode() ^ UpperLimit.GetHashCode();
}

(This is the same implementation that the Point structure uses.)

Any implementation of the method that always returns the same hash code for any given parameter values works when used in a Dictionary. Just returning the same hash code for all values actually also works, but then the performance of the Dictionary gets bad (looking up a key becomes an O(n) operation instead of an O(1) operation. To give the best performance, the method should distribute the hash codes evenly within the range.

If your data is strongly biased, the above implementation might not give the best performance. If you for example have a lot of ranges where the lower and upper limits are the same, they will all get the hash code zero. In that case something like this might work better:

public int GetHashCode() {
   return (LowerLimit.GetHashCode() * 251) ^ UpperLimit.GetHashCode();
}

You should consider making the class immutable, i.e. make it's properties read-only and only setting them in the constructor. If you change the properties of an object while it's in a Dictionary, it's hash code will change and you will not be able to access the object any more.