GetHashCode breaks code

Question

In my code I have the following class:

public class ResourcePack
{
    private int m_pirates;
    private int m_islands;
    private int m_enemy_drones;
    private int m_enemy_pirates;

    public ResourcePack()
    {
        this.m_pirates = 0;
        this.m_islands = 0;
        this.m_enemy_drones = 0;
        this.m_enemy_pirates = 0;
    }

    public ResourcePack(ResourcePack other)
    {
        this.m_pirates = other.m_pirates;
        this.m_islands = other.m_islands;
        this.m_enemy_drones = other.m_enemy_drones;
        this.m_enemy_pirates = other.m_enemy_pirates;
    }

    public bool CanConcatinate(ResourcePack other)
    {
        if ((this.m_islands & other.m_islands) != 0)
        {
            return false;
        }

        if ((this.m_enemy_drones & other.m_enemy_drones) != 0)
        {
            return false;
        }

        if ((this.m_enemy_pirates & other.m_enemy_pirates) != 0)
        {
            return false;
        }

        return true;
    }

    internal void AddIsland(int island)
    {
        m_islands |= (1 << island);
    }

    internal void AddPirate(int pirate)
    {
        m_pirates |= (1 << pirate);
    }

    internal void AddEnemyDrone(int drone)
    {
        m_enemy_drones |= (1 << drone);
    }

    internal void AddEnemyPirates(int pirate)
    {
        m_enemy_pirates |= (1 << pirate);
    }

    public ResourcePack SemiConcatinate(ResourcePack other)
    {
        var ret = new ResourcePack();

        ret.m_pirates       = this.m_pirates | other.m_pirates;
        ret.m_islands       = this.m_islands | other.m_islands;
        ret.m_enemy_drones  = this.m_enemy_drones | other.m_enemy_drones;
        ret.m_enemy_pirates = this.m_enemy_pirates | other.m_enemy_pirates;

        return ret;
    }

    public override bool Equals(object value)
    {
        ResourcePack other = value as ResourcePack;

        return !object.ReferenceEquals(null, other)
            && int.Equals(m_pirates, other.m_pirates)
            && int.Equals(m_islands, other.m_islands)
            && int.Equals(m_enemy_drones, other.m_enemy_drones)
            && int.Equals(m_enemy_pirates, other.m_enemy_pirates);
    }

    /*
    public override int GetHashCode()
    {
        unchecked
        {
            int hash = (int)2166136261;
            hash = (hash * 16777619) ^ m_pirates.GetHashCode();
            hash = (hash * 16777619) ^ m_islands.GetHashCode();
            hash = (hash * 16777619) ^ m_enemy_drones.GetHashCode();
            hash = (hash * 16777619) ^ m_enemy_pirates.GetHashCode();
            return hash;
        }
    }*/

    public static bool operator ==(ResourcePack a, ResourcePack b)
    {
        if (object.ReferenceEquals(a, b))
            return true;

        if (object.ReferenceEquals(null, a))
            return false;

        return a.Equals(b);
    }

    public static bool operator !=(ResourcePack a, ResourcePack b)
    {
        return !(a == b);
    }
}

There's only one place where this code is being used and once an instance of ResourcePack was created the members doesn't change anymore. The only way ResourcePack is used as is a key index for a dictionary:

Dictionary<ResourcePack, T> current = new Dictionary<ResourcePack, T>(missions[0].Length);
// current[t1] = t2; where t1 is an instance of ResourcePack

When I dont override GetHashCode, the code works as it supposed to. When I do override (Uncomment GetHashCode), the code doesn't work the same and the output seems random.

Can someone please explain me this weired behavior? Is my custom GetHashCode not good enough?

Update I've changed my code after reading your suggestions, but it still seems that GetHashCode doesn't return unique (enough?) values.

New code:

public sealed class ResourcePack
{
    private readonly int m_pirates;
    private readonly int m_islands;
    private readonly int m_enemy_drones;
    private readonly int m_enemy_pirates;

    public ResourcePack(int pirates, int islands, int enemy_drones, int enemy_pirates)
    {
        this.m_pirates = pirates;
        this.m_islands = islands;
        this.m_enemy_drones = enemy_drones;
        this.m_enemy_pirates = enemy_pirates;
    }

    public ResourcePack(ResourcePack other)
    {
        this.m_pirates = other.m_pirates;
        this.m_islands = other.m_islands;
        this.m_enemy_drones = other.m_enemy_drones;
        this.m_enemy_pirates = other.m_enemy_pirates;
    }

    public bool CanConcatinate(ResourcePack other)
    {
        if ((this.m_islands & other.m_islands) != 0)
        {
            return false;
        }

        if ((this.m_enemy_drones & other.m_enemy_drones) != 0)
        {
            return false;
        }

        if ((this.m_enemy_pirates & other.m_enemy_pirates) != 0)
        {
            return false;
        }

        return true;
    }

    public ResourcePack SemiConcatinate(ResourcePack other)
    {
        return new ResourcePack(this.m_pirates | other.m_pirates,
            this.m_islands | other.m_islands,
            this.m_enemy_drones | other.m_enemy_drones,
            this.m_enemy_pirates | other.m_enemy_pirates);
    }

    #region Hashing

    public override bool Equals(object value)
    {
        ResourcePack other = value as ResourcePack;

        return !object.ReferenceEquals(null, other)
            && int.Equals(m_pirates, other.m_pirates)
            && int.Equals(m_islands, other.m_islands)
            && int.Equals(m_enemy_drones, other.m_enemy_drones)
            && int.Equals(m_enemy_pirates, other.m_enemy_pirates);
    }

    public override int GetHashCode()
    {
        unchecked
        {
            int hash = (int)23;
            hash = (hash * 17) ^ m_pirates.GetHashCode();
            hash = (hash * 17) ^ m_islands.GetHashCode();
            hash = (hash * 17) ^ m_enemy_drones.GetHashCode();
            hash = (hash * 17) ^ m_enemy_pirates.GetHashCode();
            return hash;
        }
    }

    public static bool operator ==(ResourcePack a, ResourcePack b)
    {
        if (object.ReferenceEquals(a, b))
            return true;

        if (object.ReferenceEquals(null, a))
            return false;

        return a.Equals(b);
    }

    public static bool operator !=(ResourcePack a, ResourcePack b)
    {
        return !(a == b);
    }

    #endregion
}

If I remove the region called "Hashing" everything works fine.

Answer 1

GetHashCode must return the same value while the object is used within the same Dictionary. See the documentation for more details

Dictionary<TKey,TValue> uses the HashCode in order to find a bucket for the Key passed when adding, getting and removing elements.

Altering the Hashcode between Writing and Reading to the Dictionary will make it pick the wrong bucket to look for the element and thus giving you unexpected results.

If its not super performance critical, why not make use of some existing things, e.g. Tuple's GetHashCode

Tuple.Create(m_pirates,m_islands,m_enemy_drones,m_enemy_pirates).GetHashCode();

Answer 2

When you don't override GetHashCode, the dictionary is indexed by the object reference (the sync block). There is no way to create a new instance of your key if it is lost.

On the other hand, your implementation of both GetHashCode and Equals makes it possible to "recreate" the key just by using the same fields. At the same time, however, it means that your ResourcePack keys are no longer unique by default - if they have the same fields, they will map to the same item in the dictionary.

The most likely causes for the issues you're seeing are:

• You mutate the hashcode fields.
• You have two (or more) ResourcePack objects that are supposed to be different, but actually have the same fields. The default implementation keeps them unique, yours doesn't.