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.
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:
ResourcePack
objects that are supposed to be different, but actually have the same fields. The default implementation keeps them unique, yours doesn't.