0

I have a custom collection as shown below

public class CustomCollection<T>:IEnumerable<T>, IEnumerator<T>
{
    int size = 0;
    int current = 0;
    int position = -1;
    CustomComparer<T> cmp = new CustomComparer<T>();

    T[] collection = null;
    public CustomCollection(int sizeofColl)
    {
        size = sizeofColl;
        collection = new T[size];
    }

    public void Push(T value)
    {
        if (!collection.Contains(value, cmp))
            collection[current++] = value;
    }

    public T Pop()
    {
        return collection[--current];
    }        

    IEnumerator<T> IEnumerable<T>.GetEnumerator()
    {
        return (IEnumerator<T>)this;
    }

    System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator()
    {
        throw new NotImplementedException();
    }

    public T Current
    {
        get { return collection[position]; }
    }

    public void Dispose()
    {

    }

    object System.Collections.IEnumerator.Current
    {
        get { throw new NotImplementedException(); }
    }

    public bool MoveNext()
    {
        position++;
        if (position >= collection.Length)
            return false;
        else
            return true;
    }

    public void Reset()
    {
        throw new NotImplementedException();
    }
}

Now I want to have a collection of Person class which is as below along with the IEqualityComparer

 public class Person
{
    public string Name { get; set; }
    public int ID { get; set; }       
}

public class CustomComparer<T>:IEqualityComparer<T>    {


    public bool Equals(T x, T y)
    {
        Person p1 = x as Person;
        Person p2 = y as Person;
        if (p1 == null || p2 == null)
            return false;
        else
            return p1.Name.Equals(p2.Name);
    }

    public int GetHashCode(T obj)
    {
        Person p = obj as Person;
        return p.Name.GetHashCode();
    }
}

Now when I perform the following operation on the collection, why only Equals Method is called and not the GetHashCode() ?

  CustomCollection.CustomCollection<Person> custColl = new CustomCollection<Person>(3);
        custColl.Push(new Person() { Name = "per1", ID = 1 });
        custColl.Push(new Person() { Name = "per2", ID = 2 });
        custColl.Push(new Person() { Name = "per1", ID = 1 });

Or how can I make my code to call GetHashCode ?

Vikram
  • 1,617
  • 5
  • 17
  • 37
  • Recommended reading: http://stackoverflow.com/questions/371328/why-is-it-important-to-override-gethashcode-when-equals-method-is-overridden – Richard Mar 07 '13 at 10:06
  • Additionally, in your logic for your `Equals(...)`, what happens if **both** names are null? Should these be considered equal? Or should the exception be thrown. – Richard Mar 07 '13 at 10:08
  • @Richard I think it would be better to throw an exception – Vikram Mar 07 '13 at 10:11
  • 1
    @Vikram meh; personally I'd just use `return p1.Name == p2.Name` - job done and no issues with `null`. Also; you might want to consider what happens if both `x` and `y` are `null`. Most people would expect that to `return true;` – Marc Gravell Mar 07 '13 at 10:13
  • 2
    Btw, .NET has a [`Stack`](http://msdn.microsoft.com/en-us/library/3278tedw(v=vs.90).aspx) class already. And why do you actually want `GetHashCode` to be called? – vgru Mar 07 '13 at 10:14
  • If you are implementing a comparer for a `Person` class, then declare it as `PersonComparer : IEqualityComparer`. What you are doing now doesn't make sense (using generics, and then casting to a specific class). – vgru Mar 07 '13 at 10:20
  • @Groo Thanks a lot for your advice. I Want GetHashCode to be called, because if I am not wrong it improves the performance if the collection is big – Vikram Mar 07 '13 at 10:22
  • @Vikram: it's not `GetHashCode` vs `Equals` that improves performance. It's using a data structure (e.g. a `Dictionary`) which can utilize `GetHashCode` to narrow down the search space. In your case, you are using `Array.Contains`, which takes `O(n)` time complexity (it iterates through all elements). You will probably get a much better suggestion if you write what you are actually trying to do with this structure. Are you sure that this structure has to work like a **stack** (LIFO)? – vgru Mar 07 '13 at 10:27

1 Answers1

2

This relates to the line:

if (!collection.Contains(value, cmp))

A test against a vector or sequence (since that looks like Enumerable.Contains) would have no purpose in calling GetHashCode(); that is useful if the data has been grouped into hash-buckets or some other optimized structure, but the data here is just a flat sequence of values. If it needs to call a method, it might as well call Equals rather than GetHashCode(), because if the hash was the same it would still need to call Equals (a hash-code indicates non-equality, but cannot indicate equality). So it is a choice of calling exactly one method per object, vs at least one method per object, and possibly two methods per object. The first is obviously preferable.

If the data were a Dictionary<Person, ...> or a HashSet<Person>, then I would expect GetHashCode() to be used.

Marc Gravell
  • 1,026,079
  • 266
  • 2,566
  • 2,900
  • In some cases, it may make fast for collections that aren't hash tables to cache the hash values of their contents, and use a hash comparison as a prelude to a more detailed comparison. Searching an array of integers for a value is O(1), just like searching through a list of values for one that returns true for `Equals`, but could be many times faster. – supercat Mar 07 '13 at 17:28
  • @supercat that basically depends on *having an implementation* to do that. A vector/array ***does not*** - the data here is in a vector. So searching an array of integers for a value is always going to be `O(n)`. I agree there are scenarios where hashing is used, but since the code *clearly* shows we are using a vector directly: ***this is not one of them***. – Marc Gravell Mar 08 '13 at 07:37