6

We're using a class library that performs calculations on 3D measurement data, it exposes a method:

MeasurementResults Calculate(IList<IList<Measurement>> data)

I would like to allow calling this method with any indexable list of lists (of Measurement of course), for example both:

Measurement[][] array;
List<List<Measurement>> list;

Calling the method using the array works fine, which is a bit strange. Is there some compiler trick at work here? Trying to call with the List gives the familiar error:

cannot convert from 'List<List<Measurement>>' to 'IList<IList<Measurement>>'

So, I have written a facade class (containing some other things as well), with a method that splits the generic definition between the argument and method and converts to the IList type if necessary:

MeasurementResults Calculate<T>(IList<T> data) where T : IList<Measurement>
{
  IList<IList<Measurement>> converted = data as IList<IList<Measurement>>;
  if(converted == null)
    converted = data.Select(o => o as IList<Measurement>).ToList();
  return Calculate(converted);
}

Is this a good way to solve the problem, or do you have a better idea?

Also, while testing different solutions to the problem, I found out that if the class library method had been declared with IEnumerable instead of IList, it is ok to call the method using both the array and the List:

MeasurementResults Calculate(IEnumerable<IEnumerable<Measurement>> data)

I suspect that there is some compiler trick at work again, I wonder why they haven't made IList work with List while they were at it?

Anlo
  • 3,228
  • 4
  • 26
  • 33
  • 1
    Calling the `IList>` method with a `T[][]` works because of broken array covariance. It is broken because the compiler would allow you to assign a `List` to an element of the `IList>`, but if the object is really a `T[][]`, the assignment will fail at run time. – phoog Apr 19 '12 at 14:54
  • @phoog: Aha, interesting, didn't know about that. – Anlo Apr 19 '12 at 17:25

3 Answers3

6

It's okay to do this with IEnumerable<T>, because that is covariant in T. It wouldn't be safe to do that with IList<T>, as that is used for both "input" and "output".

In particular, consider:

List<List<Foo>> foo = new List<List<Foo>>();
List<IList<Foo>> bar = foo;
bar.Add(new Foo[5]); // Arrays implement IList<T>

// Eh?
List<Foo> firstList = foo[0];

See MSDN for more information about generic covariance and contravariance.

Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
  • 1
    And of course, declaring the method with `IList>` and calling it with `T[][]` works because of broken array covariance. – phoog Apr 19 '12 at 14:51
3

Suppose we have a method

void Bar(IList<IList<int>> foo)

Within Bar, it should then be perfectly admissible to add a int[] to foo - after all, int[] implements IList<int>, doesn't it?

But if we had called Bar with a List<List<int>>, we would now be trying to add a int[] to something that only accepts a List<int>! which would be bad. So the compiler doesn't let you do this.

Also, while testing different solutions to the problem, I found out that if the class library method had been declared with IEnumerable instead of IList, it is ok to call the method using both the array and the List:

Indeed, because if the behavioural contract just says "I can output ints", nothing can go wrong. The key terms for further research are covariance and contravariance, and no one* can ever remember which is which.

In your particular case, if Calculate is only ever reading its input, changing it to consume IEnumerable is absolutely the right thing to do - it both allows you to pass in any qualifying objects, and it further communicates to anyone reading the signature that this method is intentionally designed to only consume, not mutate, its input.


* well, mostly no one

AakashM
  • 62,551
  • 17
  • 151
  • 186
  • An easy way to remember covariance versus contravariance is to think about what happens when things nest. If `Foo` is *co*variant, the relationship between `Foo` and `Foo` will be the same as the relationship between `T` and `U`. If `Foo` is *contra*variant, the relationship will be opposite. Interestingly, if `Foo` is either covariant or contravariant with respect to `T`, `Foo>` will be covariant (a routine which is covariant with respect to `T` is generally one that supplies `T`'s; a routine which accepts consumers of type `T` is expected to supply `T`'s to them). – supercat Apr 19 '12 at 15:59
  • Of course, that makes sense. Calculate do only read the data, but I'm hesitant to changing it to IEnumerable since we need random access using the indexing property. We could do a `.ToList()` inside Calculate, but that would cause all the data to be copied to new lists which might cause performance problem instead. It seems like no readonly indexable collection interface exists? – Anlo Apr 19 '12 at 16:51
  • @Anlo now that's a good question! but [this](http://stackoverflow.com/questions/3141748/indexable-interface) and [this](http://stackoverflow.com/questions/2176246/is-there-a-net-collection-interface-that-prevents-adding-objects) suggest the answer is no. By all means create your own `IReadOnlyList<>`, though. – AakashM Apr 19 '12 at 16:53
1

There are a few things with you implementation that I would change. First of all it might insert nulls into the result where there were objects in the original. Personally I would prefer it to throw.

MeasurementResults Calculate<T>(IList<T> data) where T : IList<Measurement>
{
    return Calculate(data as IList<IList<Measurement>>
                     ?? data.Cast<IList<Measurement>>().ToList());
}

Is a short example a like what I would personally do in your case, though I doubt I would actually implement the method. The interface was written like that for a reason (I hope) and I would try to use that knowledge in my implementation on top instead of fighting it if possible. Any changes made to the list in the method will not be reflected in the original list if you use your code (or anything like that). It's a new list being passed to the method and since the signature asks for an IList it's possible that changes can be made.

Aside from not potentially having nulls in the list instead of objects I've change to use the cast method since that's basically what you are trying to accomplish. (Cast does not allow for custom conversion operators).

BenMorel
  • 34,448
  • 50
  • 182
  • 322
Rune FS
  • 21,497
  • 7
  • 62
  • 96
  • Yes, throwing exceptions instead of silently exchanging objects for null values is definately a good idea. But are you sure my code can introduce null values in the list? The `where T : IList` should prohibit anything not castable to `IList`, or have I missed something? Also, I haven't really thought about the fact that a new list is passed down to Calculate. In this case that was not a problem, since the data is only read by the method - but it sure could matter in the general case. – Anlo Apr 19 '12 at 16:58
  • Also, the Cast is a lot cleaner and more informational than my Select. – Anlo Apr 19 '12 at 17:00
  • 1
    @anlo I must admit I forgot about the constraint and only read the code but then I guess that's a good example of why information scattering leads to misunderstanding (a completely different topic but very related to an area of work for me called DCI) so thks for a nice example there :) – Rune FS Apr 20 '12 at 09:10