5

I have a bit of code with the following logic:

//pseudo-code
foreach (element in elementList) {
    if (element is whatever)
        return element;
    }
}

In theory, there is always one element that is whatever, so this method should pose no problems. In any case, I've put an assertion on the end of the method just to be sure:

//pseudo-code
foreach (element in elementList) {
    if (element is whatever)
        return element;
    }
}

Contract.Assert(false, "Invalid state!");

The problem is that as this method has to return something, and the compiler doesn't understand that the assertion will break the program execution. Before using Contracts, in these kind of situations, I used to throw an Exception, which solved the problem. How would you handle this with Contract.Assert()? Returning null or default(element_type) after the Contract.Assert() call knowing it will never be called and shutting up the compiler? Or is there any other more elegant way of doing this?

Thanks

devoured elysium
  • 101,373
  • 131
  • 340
  • 557

2 Answers2

2

You could go with

var result = null;
foreach (element in elementList) {
    if (element is whatever)
        result = element;
        break;
    }
}

Contract.Assert(result != null, "Invalid state!");
return result;

It introduces a break, but it does look cleaner around the return.

Even cleaner would be

return elementList.Where( e => e is whatever).First();

Edit as @devoured pointed out the above would hit the whole list

cleaner without the where

return elementList.First( e => e is whatever);

end edit

That just blows up if there is nothing found.

But if you really want the assert it could be

var results = elementList.Where( e => e is whatever);
Contract.Assert(results.Count() == 1, "Boo");
return results.First();

but that will iterate the whole list too.

Mike Two
  • 44,935
  • 9
  • 80
  • 96
  • About using LINQ's First, will that run the whole elementList, or it stops searching for more elements in the moment it finds the first? – devoured elysium May 06 '10 at 07:07
  • 1
    @devoured I think as I wrote it will run the whole list. But perhaps using it as `elementList.First(e => e is whatever)` (note the lack of `Where`) should stop at the first one. But I haven't tested it – Mike Two May 06 '10 at 07:10
  • Oooooooops. I misclicked the "Accept answer button" and down-voted you. You have to edit your answer so it allows me to change the vote :( – devoured elysium May 06 '10 at 07:17
  • @devoured - I've done that before. Thanks for letting me know – Mike Two May 06 '10 at 07:22
  • 1
    it will not iterate the whole list. Where is lazy, and on request, just iterates till it finds an item which fullfills the predicate. – Dykam May 31 '10 at 10:31
  • @MikeTwo why not use `Contract.Ensures`? –  Nov 13 '12 at 09:14
1

I think you'd be better off with a Contract.Requires (i.e. a precondition) than a Contract.Assert here. The precondition for your snippet is that elementList contains at least one element where the condition holds.

You should be explicit about that rather than rely on an Assert(false), which requires the reader to understand the rest of the function and follow the flow control to deduce the situations in which the Assert(false) is reached.

So, I'd rewrite as:

//precondition (checked only in debug builds)
Contract.Require(elementList.Where(e => e is whatever).Count > 0,"elementList requires at least one element meeting condition whatever");
//do work (throws in release builds, never reached in debug ones)
return elementList.First( e => e is whatever);
JoeG
  • 12,994
  • 1
  • 38
  • 63
  • -, so you'll have to iterate over the list twice ... and that's not the cherry on top ... you even are using `.Count() > 0` (property call is not possible as the return of `.Where()` is `IEnumerable`, and that type has no property `.Count` but rather an extension-method ... `Enumerable.Count()`) which needs to iterate over the whole result - rather use `.Any()` –  Nov 02 '12 at 06:51
  • Nope. Someone doesn't understand how contracts work. Whenever speed is important my answer iterates over the list at most once. The precondition makes the code more readable, aids debugging, and serves as a hint to the optimiser. It is not executed in production code. – JoeG Nov 13 '12 at 03:38
  • Nope. Someone doesn't understand how Linq works (please read http://stackoverflow.com/questions/305092/which-method-performs-better-any-vs-count-0), and that the property `Count` does not exist on an `IEnumerable` - nevertheless, execution of `Contract.Require` is a matter of configuration (aka `Perform Runtime Contract Checking`: **`Full`**, `Pre and Post`, ...). –  Nov 13 '12 at 07:07