views:

376

answers:

2

The following code throws an InvalidCastException.

public static MachineProductCollection MachineProductsForMachine(
    MachineProductCollection MachineProductList, int MachineID)
{
    return (MachineProductCollection)
        MachineProductList.FindAll(c => c.MachineID == MachineID);
}

This surprises me since MachineProductCollection is merely a generic List of MachineProducts which is exactly what FindAll() should return. Here’s the full MachineProductCollection source code. You will note is merely a wrapper for List.

[Serializable]
public partial class MachineProductCollection :
        List<MachineProduct>
{
    public MachineProductCollection() { }
}

I resorted to the following which basically loops through the FindAll() result which is of type List and adds each item to my MachineProductCollection. Obviously, I don’t like the required iteration.

public static MachineProductCollection
    MachineProductForMachine(MachineProductCollection
    MachineProductList, int MachineID)
{
    MachineProductCollection result =
        new MachineProductCollection();


    foreach (MachineProduct machineProduct in
        MachineProductList.FindAll(c => c.MachineID == MachineID))
    {
        result.Add(machineProduct);
    }

    return result;
}

Documentation states an InvalidCastException is thrown when a failure occurs during an explicit reference conversion. Reference conversions are conversions from one reference type to another. While they may change the type of the reference, they never change the type or value of the conversion’s target. Casting objects from one type to another is a frequent cause for this exception.

Considering List is MachineProductCollection’s base, should this really be an InvalidCastException?

+4  A: 

Yes, the invalid cast exception is correct. You can freely cast from a derived class to a base class but you cannot blindly cast from a base class to a derived class unless the object really is an instance of the derived class. It's the same reason you cannot do this:

object obj = new object();
string str = (string) obj;

Right? object is string's base, and you cannot freely cast from object to string. On the other hand this would work since obj is indeed a string:

object obj = "foo";
string str = (string) obj;
John Kugelman
Great answer - short and sweet. It makes complete sense especially now with the object/string analogy. Thanks.
Ben Griswold
+1  A: 

You're getting the InvalidCastException because a List<MachineProduct> isn't necessarily a MachineProductCollection, even though the reverse is obviously true.

The simplest solution to return an actual MachineProductCollection would be to use List<T>'s sequence constructor:

public static MachineProductCollection
    MachineProductForMachine(MachineProductCollection MachineProductList, int MachineID)
{
    List<MachineProduct> found =
        MachineProductList.FindAll(c => c.MachineID == MachineID))

    return new MachineProductCollection(found);
}

However, since you're using lambda expressions I'm guessing you have access to LINQ (via .NET 3.5 or LINQ Bridge), which means you can use the Where extension method to skip the intermediate list:

public static MachineProductCollection
    MachineProductForMachine(MachineProductCollection MachineProductList, int MachineID)
{
    IEnumerable<MachineProduct> found =
        MachineProductList.Where(c => c.MachineID == MachineID))

    return new MachineProductCollection(found);
}

That said, you might be better off just returning an IEnumerable<MachineProduct> rather than creating your own collection type. This will probably make your life easier unless you're planning to add special logic to your MachineProductCollection. And in that case, you could always just write extension methods against IEnumerable<MachineProduct> to provide that logic. Just some things to consider...

dahlbyk
+1 for all the detail and the correct answer. Sorry I couldn't mark two answers as correct.
Ben Griswold