CODEWITHSUNDEEP
c#.netlinqiequatable
swolff1978
swolff1978

Reputation: 1865

How do I get Distinct() to work with a collection of custom objects

I have followed the suggestions from this post to try and get Distinct() working in my code but I am still having issues. Here are the two objects I am working with:

    public class InvoiceItem : IEqualityComparer<InvoiceItem>
    {
        public InvoiceItem(string userName, string invoiceNumber, string invoiceAmount)
        {
            this.UserName = userName;
            this.InvoiceNumber= invoiceNumber;
            this.InvoiceAmount= invoiceAmount;
        }
        public string UserName { get; set; }
        public string InvoiceNumber { get; set; }
        public double InvoiceAmount { get; set; }

        public bool Equals(InvoiceItem left, InvoiceItem right)
        {
            if ((object)left.InvoiceNumber == null && (object)right.InvoiceNumber == null) { return true; }
            if ((object)left.InvoiceNumber == null || (object)right.InvoiceNumber == null) { return false; }
            return left.InvoiceNumber == right.InvoiceNumber;
        }


        public int GetHashCode(InvoiceItem item)
        {
            return item.InvoiceNumber == null ? 0 : item.InvoiceNumber.GetHashCode();
        }
    }


    public class InvoiceItems : List<InvoiceItem>{ }

My goal is to populate an InvoiceItems object (we will call it aBunchOfInvoiceItems) with a couple thousand InvoiceItem objects and then do:

InvoiceItems distinctItems = aBunchOfInvoiceItems.Distinct();

When I set this code up and run it, I get an error that says

Cannot implicitly convert type 'System.Collections.Generic.IEnumerable' to 'InvoiceReader.Form1.InvoiceItems'. An explicit conversion exists (are you missing a cast?)

I don't understand how to fix this. Should I be taking a different approach? Any suggestions are greatly appreciated.

Upvotes: 2

Views: 3288

Answers (5)

Rex M
Rex M

Reputation: 144152

The error has everything to do with your class InvoiceItems, which inherits from List<InvoiceItem>.

Distinct returns an IEnumerable<InvoiceItem>: InvoiceItems is a very specific type of IEnumerable<InvoiceItem>, but any IEnumerable<InvoiceItem> is not necessarily an InvoiceItems.

One solution could be to use an implicit conversion operator, if that's what you wanted to do: Doh, totally forgot you can't convert to/from interfaces (thanks Saed)

public class InvoiceItems : List<InvoiceItem>
{
    public InvoiceItems(IEnumerable<InvoiceItem> items) : base(items) { }
}

Other things to note:

  • Inheriting from List<T> is usually bad. Implement IList<T> instead.
  • Using a list throws away one of the big benefits of LINQ, which is lazy evaluation. Be sure that prefetching the results is actually what you want to do.

Upvotes: 1

Ani
Ani

Reputation: 113442

Konrad Rudolph's answer should tackle your compilation problems. There is one another important semantic correctness issue here that has been missed: none of your equality-logic is actually going to be used.

When a comparer is not provided to Distinct, it uses EqualityComparer<T>.Default. This is going to try to use the IEquatable<T> interface, and if this is missing, falls back on the plain old Equals(object other) method declared on object. For hashing, it will use the GetHashCode() method, also declared on object. Since the interface hasn't been implemented by your type, and none of the aforementioned methods have been overriden, there's a big problem: Distinct will just fall back on reference-equality, which is not what you want.

Tthe IEqualityComparer<T> interface is typically used when one wants to write an equality-comparer that is decoupled from the type itself. On the other hand, when a type wants to be able to compare an instance of itself with another; it typically implements IEquatable<T>. I suggest one of:

  1. Get InvoiceItem to implement IEquatable<InvoiceItem> instead.
  2. Move the comparison logic to a separate InvoiceItemComparer : IEqualityComparer<InvoiceItem> type, and then call invoiceItems.Distinct(new InvoiceItemComparer());
  3. If you want a quick hack with your existing code, you can do invoiceItems.Distinct(new InvoiceItem());

Upvotes: 4

Gideon Engelberth
Gideon Engelberth

Reputation: 6155

Aside from the custom class vs IEnumerable issue that the other answers deal with, there is one major problem with your code. Your class implements IEqualityComparer instead of IEquatable. When you use Distinct, the items being filtered must either implement IEquatable themselves, or you must use the overload that takes an IEqualityComparer parameter. As it stands now, your call to Distinct will not filter the items according to the IEqualityComparer Equals and GetHashCode methods you provided.

IEqualityComparer should be implemented by another class than the one being compared. If a class knows how to compare itself, like your InvoiceItem class, it should implement IEquatable.

Upvotes: 0

Jon Hanna
Jon Hanna

Reputation: 113322

Quite simply, aBunchOfInvoiceItems.Distinct() returns an IEnumerable<InvoiceItem> and you are trying to assign that to something that is not an IEnumerable<InvoiceItem>.

However, the base class of InvoiceItems has a constructor that takes such an object, so you can use this:

public class InvoiceItems : List<InvoiceItem>
{
  public InvoiceItems(IEnumerable<InvoiceItem> items)
    base(items){}
}

Then you can use:

InvoiceItems distinctItems = new InvoiceItems(aBunchOfInvoiceItems.Distinct());

As is though, I don't see much benefit in deriving from List<InvoiceItem> so I would probably lean more toward:

List<InvoiceItem> distinctItems = aBunchOfInvoiceItems.Distinct().ToList();

Upvotes: 3

Konrad Rudolph
Konrad Rudolph

Reputation: 545865

Distinct returns a generic IEnumerable<T>. It does not return an InvoiceItems instance. In fact, behind the curtains it returns a proxy object that implements an iterator that is only accessed on demand (i.e. as you iterate over it).

You can explicitly coerce it into a List<> by calling .ToList(). You still need to convert it to your custom list type, though. The easiest way is probably to have an appropriate constructor, and calling that:

public class InvoiceItems : List<InvoiceItem> {
    public InvoiceItems() { }

    // Copy constructor
    public InvoiceItems(IEnumerable<InvoiceItems> other) : base(other) { }
}

// …

InvoiceItems distinctItems = new InvoiceItems(aBunchOfInvoiceItems.Distinct());

Upvotes: 5

Related Questions