CODEWITHSUNDEEP
c#.net
Rocket254
Rocket254

Reputation: 135

Safe usage of .FirstorDefault()?

When using Enumerable.FirstorDefault(), do I need to always catch the ArumentNullException that can be thrown when the collection operated on is null?

In the past I've always just done something like this:

WorkflowColorItemType associatedColor = ColorItems
    .Where(ci => ci.AssociatedState == WorkflowStateStatus.NotStarted)
    .FirstOrDefault();

if (associatedColor != null)
{
    this.ColorItems.CurrentColor = associatedColor;
}

In the context of this code snippet, I would never expect ColorItems to be null but is it good practice to be enclosing every instance of snippets like this in try catch blocks so I can handle the off chance that the ColorItems collection might be null?

Upvotes: 3

Views: 5954

Answers (3)

DarthVader
DarthVader

Reputation: 55022

Yes, certainly. You need to be defensive all the time.

In fact, if associatedColor is null, that means there is something wrong, hence you need to handle it.

In fact, your code used to be wrapped in a try/catch block to handle exceptions, since, exceptions are "expensive", this is cheaper and nicer way to handle exceptional cases.

In any case, I would almost always use FirstOrDefault or something simirlar, like SingleOrDefault then I would do the null check.

Upvotes: 1

Servy
Servy

Reputation: 203823

If you don't expect the collection to ever be empty, and it would be an error in your program for it to be empty, then don't use FirstOrDefault in the first place, use First. Since it's not an expected situation for you to be in you want to draw attention to the problem because it's a sign that something is wrong.

If it's entirely valid for the collection to be empty, and you want to use the first item only if there is at least one item, then using FirstOrDefault and providing a null check is fine.

Apply the same logic to the collection being null, and not empty. If it is expected that the collection be allowed to be null, then check for it using an if. If it's not expected that the collection be allowed to be null (which is generally the case for most uses of collections) then you shouldn't check, and you want the exception to be thrown as it will draw attention to the bug in the code that is supposed to populate that collection. Trying to catch the exception and move on is trying to obscure the bug which prevents you from finding and fixing it.

Upvotes: 4

Charles Boyung
Charles Boyung

Reputation: 2481

The built in LINQ functions (like .Where() here) always return an empty enumerable if there are no results, not null. So there is no need to check for null after doing the .Where()

Depending on where ColorItems comes from, you should check for null on the object:

if (ColorItems != null) 
{
}
else
{
}

There's no need to put a try/catch block around the code, but you should be checking for null just to be safe. In fact, using try/catch in a scenario like this, where you can just check for a null object, is a bad programming practice.

Upvotes: 0

Related Questions