We experienced some slowness in our code opening a form and it was possibly due to a for loop with a break that was taking a long time to execute. I switched this to an IEnumerable.Any() and saw the form open very quickly. I am now trying to figure out if making this change alone increased performance or if it was accessing the Prod开发者_StackOverflow社区uctIDs property more efficiently. Should this implementation be faster, and if so, why?

Original Implementation:

public bool ContainsProduct(int productID) {
    bool containsProduct = false;
    for (int i = 0; i < this.ProductIDs.Length; i++) {
        if (productID == this.ProductIDs[i]) {
            containsProduct = true;
            break;
        }
    }
    return containsProduct;
}

New Implementation:

public bool ContainsProduct(int productID) {
    return this.ProductIDs.Any(t => productID == t);
}

Call this an educated guess:

this.ProductIDs.Length

This probably is where the slowness lies. If the list of ProductIDs gets retrieved from database (for example) on every iteration in order to get the Length it would indeed be very slow. You can confirm this by profiling your application.

If this is not the case (say ProductIDs is in memory and Length is cached), then both should have an almost identical running time.

First implementation is slightly faster (enumeration is slightly slower than for loop). Second one is a lot more readable.

UPDATE

Oded's answer is possibly correct and well done for spotting it. The first one is slower here since it involves database roundtrip. Otherwise, it is slightly faster as I said.

UPDATE 2 - Proof

Here is a simple code showing why first one is faster:

    public static void Main()
    {

        int[] values = Enumerable.Range(0, 1000000).ToArray();

        int dummy = 0;
        Stopwatch stopwatch = new Stopwatch();

        stopwatch.Start();
        for (int i = 0; i < values.Length; i++)
        {
            dummy *= i;
        }
        stopwatch.Stop();
        Console.WriteLine("Loop took {0}", stopwatch.ElapsedTicks);

        dummy = 0;
        stopwatch.Reset();
        stopwatch.Start();
        foreach (var value in values)
        {
            dummy *= value;         
        }
        stopwatch.Stop();
        Console.WriteLine("Iteration took {0}", stopwatch.ElapsedTicks);

        Console.Read();
    }

Here is output:

Loop took 12198

Iteration took 20922

So loop is twice is fast as iteration/enumeration.

I think they would be more or less identical. I usually refer to Jon Skeet's Reimplementing LINQ to Objects blog series to get an idea of how the extension methods work. Here's the post for Any() and All()

Here's the core part of Any() implementation from that post

public static bool Any<TSource>( 
    this IEnumerable<TSource> source, 
    Func<TSource, bool> predicate) 
{ 
   ...

    foreach (TSource item in source) 
    { 
        if (predicate(item)) 
        { 
            return true; 
        } 
    } 
    return false; 
} 

This post assumes that ProductIDs is a List<T> or an array. So I'm talking about Linq-to-objects.

Linq is usually slower but shorter/more readable than conventional loop based code. A factor of 2-3 depending on what you're doing is typical.

Can you refactor your code to make this.ProductIDs a HashSet<T>? Or at least sort the array so you can use a binary search. Your problem is that you're performing a linear search, which is slow if there are many products.

I think the below implementation would be a little faster than the corresponding linq implementation, but very minor though

public bool ContainsProduct(int productID) {
    var length = this.ProductIDs.Length;

    for (int i = 0; i < length; i++) {
        if (productID == this.ProductIDs[i]) {
            return true;
        }
    }

    return false;
}

The difference will be generally in memory usage then speed.

But generally you should use for loop when you know that you will be using all elements of array in other cases you should try to use while or do while.

I think that this solution use minimum resources

int i = this.ProductIDs.Length - 1;

while(i >= 0) {
 if(this.ProductIDs[i--] == productId) {
   return true;
 }
}

return false;