Graceful way to handle item not found in a collection in C#

I have the following scenario:

1. I have a foreach loop that loops through a collection, if an item is found (based on criteria as in example below), it will return that item.

If not what is the most graceful way to dea开发者_Go百科l with this exception. I have a throw new notimplementedexception but I think there is a more elegant way.

Code is:

 foreach (SPContentType type in sPContentTypeCollection)
            {
                if (type.Name == contentTypeName)
                {
                    return type; 
                }
            }

 throw new NotImplementedException(); 

As you can see, it's not what I would call readable code. How can I make it easier for the next guy to maintain. On a side note, it does what it should from a technical perspective.

Well NotImplementedException is certainly inappropriate, because you have implemented the method... but what sort of exception should you throw? It really depends on the context, to be honest.

Sometimes it would be appropriate to return null to indicate a missing value. Other times, throwing an exception is fine - possibly an InvalidOperationException for example. You should throw an exception if this situation represents an error of some description, rather than it being a perfectly reasonable situation which the caller should expect to occur.

As for the rest of the code... if you're using .NET 3.5 you could just use LINQ:

return sPContentTypeCollection.Cast<SPContentType>()
                              .First(type => type.Name == contentTypeName);

That will throw an InvalidOperationException for you automatically if the name isn't found. Or if you want to return null:

// type shortened to t to avoid scrollbars...
return sPContentTypeCollection.Cast<SPContentType>()
                              .FirstOrDefault(t => t.Name == contentTypeName);

It really depends on whether this is an expected or exceptional case.

If the item you compare with is passed to a method and there really always should be a matching item in the collection I'd throw an ArgumentOutOfRangeException exception.

If it is expected that sometimes no matching item is found, I would just return null to indicate that the requested item was not found.

NotImplementedException is definitively the wrong exception. I would rather throw a custom exception like for example ElementNotFoundInCollection.

Another approach would be to just return null. Then you have to check on the calling side if the returned object is not null.

I agree with Rofl that NotImplemented is wrong. If you don't want to define your own exception class, KeyNotFoundException is pretty close.

Returning null is also an option.

Well, it might be worth looking at how the .NET framework itself handles this case in its own classes. For example, take Dictionary<TKey, TValue>. There are two ways to get a value out of a dictionary:

1. myDic[key]

   • Advantage: short; can be assigned to (and thus can use ++ and similar operators)
   • Disadvantage: throws KeyNotFoundException if key does not exist

2. myDic.TryGetValue(key, out value)

   • Advantage: does not throw (but returns false) if key not found
   • Disadvantage: can only get value, not set

So if you only need to get a value, and you don’t want to throw an exception, then the second is clearly preferable.

Therefore, in your case, write the function similarly:

public bool TryGetType(string contentTypeName, out SPContentType result)
{
    foreach (SPContentType type in sPContentTypeCollection)
    {
        if (type.Name == contentTypeName)
        {
            result = type;
            return true;
        }
    }
    result = default(SPContentType);
    return false;
}