Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I don't know why this is not in the .NET Library, but I need to be able to use an index against a list and not have it throw an exception if it is outside the bounds of the list.

This is what I came up with:

public static class LinqHelpers
{
    public static TListType IndexOrDefault<TListType>(this List<TListType> list,
                                                      int index)
    {
        var count = list.Count();           
        if (index > count -1)
            return default(TListType);              
        if (index < 0)
            return default(TListType);          
        return list[index];
    }
}

This is my test:

List<string> somelist = new List<string> {"B", "A", "Q", "a", "t", "h"};
for (int i = -100; i < 100; i++)
{
    Console.WriteLine(somelist .IndexOrDefault(i)); 
}   

I would appreciate any advice on missing error handling or if I am reinventing an already existing feature. I also would not mind advice on a way to use an IEnumerable instead of a List with out iterating the IEnumerable more than once.

share|improve this question
    
If the Count() were an issue (it isn't, see below) you could move the if(index<0) return default(T) above that to avoid calculating the count where you don't actually need it. –  Roman Reiner Jun 18 at 22:46
add comment

1 Answer 1

up vote 6 down vote accepted
public static TListType IndexOrDefault<TListType>(this List<TListType> list, int index)

You're extending a concrete type, and the type parameter's name is misleading - it's not the type of the list, it's the type of its items. I'd just call it T. IList<T> calls it T, so I don't see a problem with T ;)

You should be extending an interface, in this case probably an IEnumerable<T>, like LINQ extension methods do.

var count = list.Count();

You're iterating the List<T> through its IEnumerable<T> interface (doesn't apply to List<T> because a List<T> is also an ICollection<T> see @svick's comment below, but in other cases that would be \$O(n)\$ already), and you haven't tried to find anything yet! A List<T> doesn't need to be iterated to retrieve its count, just use the Count property getter, like this:

var count = list.Count;

Actually this whole block:

    var count = list.Count();           
    if (index > count -1)
        return default(T);              
    if (index < 0)
        return default(T);
    return list[index];

Could be simplified to:

if (index > list.Count - 1 || index < 0)
{
    return default(T)
}

return list[index];

But that's still extending a List<T>. You're going to want to extend IEnumerable<T> instead... but the method you're implementing already exists in System.Linq:

public static T IndexOrDefault<T>(this IEnumerable<T> source, int index)
{
    return source.ElementAtOrDefault(index);
}

You're duplicating the functionality of the IEnumerable<T>.ElementAtOrDefault extension method.

share|improve this answer
    
Fantastic answer! Thank you! –  Vaccano Jun 18 at 0:27
1  
Count() actually isn't \$O(n)\$ for collections that implement ICollection<T>, because it just accesses their Count. But the advice to prefer Count over Count() still stands. –  svick Jun 18 at 0:48
    
@Mat'sMug Right, I should have checked how did it render. –  svick Jun 18 at 0:50
    
I feel like a method named IndexOrDefault should throw an exception with a negative index. To me, the name implies it will provide a value at a specified index if it exists. An invalid index is still an invalid index though. Edit - Just checked, and it seems ElementAtOrDefault doesn't care about negative indexes...still feels wrong to me though. –  Ocelot20 Jun 22 at 21:32
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.