3

I am trying to generate a listbox of items that is a concatenation of two strings. I have created a class that implements IEqualityComparer and I want to make this list distinct.

private void PopulateFamily()
    {
        var source = _mfgOrdersData
            .Select(o => new FamilySelector(o.ItemWrapper.ItemClass, o.ItemWrapper.Family))
            .Distinct()
            .OrderBy(f => f.CodeFamily)
            .ToList();

        FamilyFilterListBox.DataSource = source;
        FamilyFilterListBox.ValueMember = "Family";
        FamilyFilterListBox.DisplayMember = "CodeFamily";
    }

class FamilySelector : IEqualityComparer<FamilySelector>
{
    public FamilySelector(string code, string family)
    {
        Code = code;
        Family = family;
    }

    public string Code { get; set; }
    public string Family { get; set; }
    public string CodeFamily { get { return string.Format("{0}\t{1}", Code, Family); } }

    public bool Equals(FamilySelector x, FamilySelector y)
    {
        return x.Code == y.Code && x.Family == y.Family;
    }

    public int GetHashCode(FamilySelector obj)
    {
        return obj.Code.GetHashCode() ^ obj.Family.GetHashCode();
    }
}

The problem is that I am getting the list non distinct. The same item appears multiple times. I presume that Equals() or GetHashCode() are not implemented correctly.

ehh
  • 3,412
  • 7
  • 43
  • 91
  • If Implementing the IEqualityComparer proves difficult, there is another way: https://stackoverflow.com/a/491832/8155 – Amy B Apr 01 '18 at 08:29
  • 1
    `IEqualityComparer` is for a class whose role is to do the comparing. You'd have to pass one as the argument to `Distinct`. `IEquatable` on the other hand, is for a class whose instances can be compared for equality. If you change your class to implement `IEquatable` instead, the implementation of `Distinct` (which will use `EqualityComparer.Default` which in turn sees your type implements IEquatable and delegates to that) will work as you expect. – pinkfloydx33 Apr 01 '18 at 09:42
  • @pinkfloydx33 Your comment is the correct answer btw. OP just messed up the interface they need to implement. – Ivan Stoev Apr 01 '18 at 14:34

2 Answers2

4

Currently you run Distinct() on a collection of FamilySelector instances which results to comparing by reference equality.

To do it right, you should pass an instance of IEqualityComparer to Distinct() call:

var source = _mfgOrdersData
    .Select(o => new FamilySelector(o.ItemWrapper.ItemClass, o.ItemWrapper.Family))
    .Distinct(new FamilySelector())
    .OrderBy(f => f.CodeFamily)
    .ToList();

You should add parameterless constructor to FamilySelector class so that code could be compiled.

I'd also suggest small refactoring of FamilySelector class. Currently it holds the data and performs comparison. Usually implementation of IEqualityComparer is a data-less class that just performs a comparison:

class FamilyData
{
    public FamilyData(string code, string family)
    {
        Code = code;
        Family = family;
    }

    public string Code { get; set; }
    public string Family { get; set; }
    public string CodeFamily { get { return string.Format("{0}\t{1}", Code, Family); } }
}

class FamilySelector : IEqualityComparer<FamilyData>
{
    public bool Equals(FamilyData x, FamilyData y)
    {
        return x.Code == y.Code && x.Family == y.Family;
    }

    public int GetHashCode(FamilyData obj)
    {
        return obj.Code.GetHashCode() ^ obj.Family.GetHashCode();
    }
}

var source = _mfgOrdersData
    .Select(o => new FamilyData(o.ItemWrapper.ItemClass, o.ItemWrapper.Family))
    .Distinct(new FamilySelector())
    .OrderBy(f => f.CodeFamily)
    .ToList();
CodeFuller
  • 30,317
  • 3
  • 63
  • 79
2

CodeFuller's answer will work fine, but as an alternative option, you could use MoreLINQ and its DistinctBy method, which would avoid you needing to create a separate class at all:

private void PopulateFamily()
{
    var source = _mfgOrdersData
        .Select(o => o.ItemWrapper)
        .DistinctBy(o => new { o.ItemClass, o.Family })
        .OrderBy(f => f.CodeFamily)
        .ToList();

    FamilyFilterListBox.DataSource = source;
    FamilyFilterListBox.ValueMember = "Family";
    FamilyFilterListBox.DisplayMember = "CodeFamily";
}

I've assumed that you want to select ItemWrapper - it's hard to tell without seeing the types involved, but that looks likely to be what you want.

Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194