Custom key class not present in dictionary

Question

I'm trying to figure out how to use custom class as key in dictionary. My class

public class Enabler
{
    public string Name { get; set; }
    public bool Enabled { get; set; }

    public override int GetHashCode()
    {
        return Name.GetHashCode() ^ Enabled.GetHashCode();
    }

    public override bool Equals(object obj)
    {
        return Equals(obj as Enabler);
    }

    public bool Equals(Enabler obj)
    {
        return obj != null && obj.Name == this.Name;
    }
}

And when I try to use it:

Dictionary<Enabler, List<AppSettingsElement>> appSettings = new Dictionary<Enabler, List<AppSettingsElement>>();
appSettings.Add(new Enabler {Name = "none", Enabled = true }, new List<AppSettingsElement>());
Enabler myKey = new Enabler { Name = "none" };
appSettings[myKey].Add(new AppSettingsElement { Comment = text, Name = name, Value = "value" });
//last line throws "myKey not found in dictionary" error.

What am I doing wrong?

Your `Equals` method should match `GetHashCode` - right now, `Enabled` is considered by only one of them. Note: mutable keys are a *terrible* idea — Marc Gravell, Mar 02 '18 at 11:45
@MarcGravell I don't quite understand what int should I return there, can you please elaborate? — Kajbo, Mar 02 '18 at 11:48

Marc Gravell · Accepted Answer · 2018-03-02T11:54:12.547

Your Equals and GetHashCode methods need to be compatible. Right now, your GetHashCode method considers Enabled, but Equals does not, which can lead to things being unfindable. Either remove the Enabled check from GetHashCode:

    public override int GetHashCode()
    {
        return Name.GetHashCode();
    }

or add it to Equals:

    public bool Equals(Enabler obj)
    {
        return obj != null && obj.Name == this.Name
            && obj.Enabled == this.Enabled;
    }

noting that with the second option, myKey will need to have Enabled = true to be a match.

Note: mutable keys are a terrible terrible idea. You should consider:

public Enabler(string name, bool enabled)
{
    Name = name;
    Enabled = enabled;
}
public string Name { get; }
public bool Enabled { get; }

to make it immutable.

Note: if you make it immutable, it might also make sense for this to be a struct, so:

public struct Enabler
{ ... }

and fixing the null checks:

public override bool Equals(object obj)
    => Equals(obj is Enabler e && Equals(e));

public bool Equals(Enabler obj)
    => obj.Name == this.Name && obj.Enabled == this.Enabled;

Either way, you should also make Enabler implement IEquatable<Enabler>, i.e.

public class Enabler : IEquatable<Enabler>

or

public struct Enabler : IEquatable<Enabler>

@Evk on `Name`? yes, probably - but arguably it would be better to check that in the constructor, i.e. `Name = name ?? throw new ArgumentNullException(nameof(name));` — Marc Gravell, Mar 02 '18 at 12:24

Custom key class not present in dictionary

1 Answers1