views:

119

answers:

1

Say we have a method that looks like this:

public IEnumerable<Dog> GrowAll(this IEnumerable<Puppy> puppies)
{
    if(subjects == null)
        throw new ArgumentNullException("subjects");

    foreach(var puppy in puppies)
        yield return puppy.Grow();
}

If I test that by doing this:

Puppy[] puppies = null;
Assert.Throws<ArgumentNullException>(() => puppies.GrowAll());

The test will fail saying that it

Expected: <System.ArgumentNullException>
But was: null

I can fix that by changing the test to

Puppy[] puppies = null;
Assert.Throws<ArgumentNullException>(() => puppies.GrowAll().ToArray());

Is this just how you would usually do it? Or is there a better way to write the test? Or maybe a better way to write the method itself?


Tried to do the same with the built-in Select method, and it failed even without a ToArray or anything like that, so apparently there is something you can do about it... I just don't know what :p

+1  A: 

The test is fine - your code isn't. You should make the code throw the exception as soon as it's called, by splitting the method in half:

public IEnumerable<Dog> GrowAll(this IEnumerable<Puppy> puppies)
{
    if(subjects == null)
        throw new ArgumentNullException("subjects");

    return GrowAllImpl(puppies);
}

private IEnumerable<Dog> GrowAllImpl(this IEnumerable<Puppy> puppies)
{
    foreach(var puppy in puppies)
        yield return puppy.Grow();
}
Jon Skeet
Aaaah, so that's why all your methods look like that in that library of yours... Makes sense! Not sure if I like it though... hehe. Must say I prefer to keep it in one method. But oh well. I suppose I will get used to it after a while :)
Svish
It's an unfortunate side-effect of iterator blocks. See http://msmvps.com/blogs/jon_skeet/archive/2008/03/02/c-4-idea-iterator-blocks-and-parameter-checking.aspx
Jon Skeet
Exactly. Yeah, I kind of understood why it happened. Just not how the recommended way to deal with it was :)
Svish