9 Responses for "Comparing two arrays (or IEnumerables) in C#"

1. Jurgen March 18th, 2008 at 8:56 pm 1

   Don’t know whether comparing two IEnumerable has a real use but ok.

   One point though : maybe you should avoid the use of Count() extension method on IEnumerable.
   As you surely know Count() tries to convert the IEnumerable to a ICollection first but if that conversion fails, il enumerates on the IEnumerable.
   In my opinion that wouldn’t be very efficient : you already enumerate the IEnumerable so why would you do it twice for each of them ?

   So I would get rid of the lines 15-18 and fix line 24 in order to be able to receive IEnumerable with different “lengths”.

   Of course then, you may want to speed up the evaluation of the comparison if _both_ IEnumerable are ICollection.

   I have no 3.5 compiler here (and though am unsure of what follows) but isn’t it possible to call an extension method with a null “this” ? (I think I may call the method without the extension-method thingee, no ? Just calling Namespace.Class.IsEqualTo((IEnumerable)null, (IEnumerable)null); ) If this is the case then your code misses a check.
   Moreover :
   * what about (value == compareList) ? No need to enumerate then…
   * as an extension, you haven’t defined what happens when both IEnumerable are null : true of false ?

   Despite what I’ve said, this is a good try and please see no offense in this comment (remember it’s easier to comment than to produce) : none of my .NET developers colleagues here is able to write such code (they don’t know about generics and extension methods).

   By the way, when you test a method that receives IEnumerable, don’t forget to test with a real IEnumerable that has no Count property (ie not an array or ICollection). As an example you may just wrap up your collection with the (untested) method :
   static IEnumerable GetIEnumerableOverIEnumerable(IEnumerable enumerable)
   {
   if(enumerable == null) return null;
   foreach(T item in enumerable)
   yield return item;
   }
   Hope you see my point.
   Doing so you will (in theory) see that your test with two collections of 10,000 items takes a bit longer.

2. Glenn Slaven March 19th, 2008 at 12:05 am 2

   Jurgen, thanks for your comments,honestly, I’d forgotten about the Count() method enumerating over IEnumerable if the cast failed, good point. I’d also forgotten about the ability to call the method on null. Noob mistake, but that’s why I like publicly releasing code.

   I’ve updated the code with the changes.

   If both IEnumerables are null I’m returning true, based on the idea that .net returns true for (null == null). Obviously if either but not both are null, then return false. Also I’m no longer checking count, I’m just enumerating over the collections until one of them runs out, then checking whether either still has more items. If either do, then it returns false, as they can’t be equal. If not return true.

   In reference to your first point, as to why IEnumerable, I’m guessing you’re asking why not just ICollection. I guess I did IEnumerable because I like making things as decoupled as possible. It can compare iEnumerables, so we may as well. I’ve run some test now comparing 2 strings, which inherit from IEnumerable and it works fine.

3. Jurgen March 19th, 2008 at 1:12 am 3

   Glenn,

   [As generic types are "eaten" by the commenting system, I would replace <T> with (T).. I don't like that kind of VBish syntax, but anyway...]

   Sorry I wasn’t clear enough: I wasn’t wondering why you use IEnumerable(T) as your interface; that’s good and consistent with what the .NET Framework already exposes.

   What I was trying to say is to perform something like :

   /* Some tests keep being done before */
   /* … */
   /* Try cast as ICollection(T) */
   ICollection(T) valueCollection = value as ICollection(T);
   ICollection(T) compareListCollection = compareList as ICollection(T);
   /* If both are collections and have different count, then they are not equal */
   if((valueCollection!=null) && (compareListCollection!=null) && (valueCollection.Count != compareListCollection.Count))
   return false;
   /* … */
   /* Continue with rest of code */

   I don’t know whether this might be a good idea… I guess it depends on the “length” of the IEnumerables ! I think I’d personally put that code in the method, but that may be a bad choice (won’t be the first of my life !!)

   I’ve read your updated code and I see a new mistake. I’ll try to explain it before giving you a solution. You “MoveNext” the enumerators in your while condition. If one MoveNext fails, your while loop is over. Great, that’s expected !
   But just after that you’re doing another MoveNext on the same enumerator. That’s wrong; your code will say that strings “123″ and “12″ are equals (in this order) :
   “123″.IsEqualTo(”12″); // returns true; false expected.

   Hope you see my point.

   Here’s a fix based on your solution. I only performed a few tests.
   Of course, feel free to comment, use, etc. I’m your guest !

   public static bool IsEqualTo(T) (IEnumerable(T) value, IEnumerable(T) compareList, IEqualityComparer(T) comparer)
   {
   if (comparer == null)
   {
   comparer = EqualityComparer(T).Default;
   }

   if (value == null && compareList == null)
   {
   return true;
   }
   else if (value == null || compareList == null)
   {
   return false;
   }
   else
   {
   IEnumerator(T) enumerator1 = value.GetEnumerator();
   IEnumerator(T) enumerator2 = compareList.GetEnumerator();

   bool enum1HasValue = enumerator1.MoveNext();
   bool enum2HasValue = enumerator2.MoveNext();
   while (enum1HasValue && enum2HasValue)
   {
   if (!comparer.Equals(enumerator1.Current, enumerator2.Current))
   {
   return false;
   }
   enum1HasValue = enumerator1.MoveNext();
   enum2HasValue = enumerator2.MoveNext();
   }

   return !(enum1HasValue || enum2HasValue);
   }
   }

   Regards.

4. Glenn Slaven March 19th, 2008 at 8:47 am 4

   Jurgen, I’ve tested the function with different length strings & it returns false as expected. The reason is that it keeps looping until either enumerators has no more items, then it checks if both have no more items. If they both have no more, they’re the same length, so return true. If the other enumerator has more items then they’re different lengths, so it returns false.

   Both the following tests pass:

   [Test]
   public void TestDifferentLengthStrings()
   {
   string a1 = “fi”;
   string a2 = “fish”;

   Assert.IsFalse(a1.IsEqualTo(a2));
   }

   [Test]
   public void TestDifferentLengthStringsReverse()
   {
   string a1 = “fish”;
   string a2 = “fi”;

   Assert.IsFalse(a1.IsEqualTo(a2));
   }

5. Jurgen March 19th, 2008 at 10:14 pm 5

   Glenn,

   I’m ok to say “fi” != “fish” (no matter what order).
   However, did you try “fish”.IsEqualTo(”fis”); ? [or to take my example "123".IsEqualTo("12");]
   I did with the latest version of the code of your post above (in .NET2.0; I removed the “this” in the method parameters). It returns true.

   When the enumerators “lengths” differ by 1 element and the extension method is run on the “bigger” enumerator, the problem arises. The sequence is the following :
   A- looping on common elements. Ok.
   B- enumerator2 runs out of elements (MoveNext == false) while enumerator1 points to the last element of the “bigger” enumerator.
   C- the while condition is thus false. Moving to return statement.
   D- enumerator2.MoveNext still remains false.
   E- enumerator1.MoveNext becomes false because it’s now pointing after the last element. We’ve sort of ignored the last element of enumerator1 (we made twice MoveNext on it : at steps B then E).
   F- Return statement thus returns true.

   To solve this issue you :
   - either have to MoveNext() transactionnally both enumerators in the while condition. I don’t know how to do this properly in an elegant way…
   - or have to store the result of the MoveNext() as I did in the previous code I posted. These values are the states of the last calls to MoveNext(). I can therefore use them in the return statement, without “MoveNexting” the enumerator…

   As an alternative to my code, you may also want to remove additional MoveNext() calls in the body. This gives the following code (we have here one single call MoveNext for each enumerator) :

   public static bool IsEqualTo<T>(IEnumerable<T> value, IEnumerable<T> compareList, IEqualityComparer<T> comparer)
   {
   /* begin of method goes here */
   else
   {
   IEnumerator<T> enumerator1 = value.GetEnumerator();
   IEnumerator<T> enumerator2 = compareList.GetEnumerator();

   bool enum1HasValue;
   bool enum2HasValue;
   while ((enum1HasValue=enumerator1.MoveNext()) & (enum2HasValue=enumerator2.MoveNext()))
   {
   if (!comparer.Equals(enumerator1.Current, enumerator2.Current))
   {
   return false;
   }
   }
   return !(enum1HasValue || enum2HasValue);
   }
   }

   Note that this last snippet uses a non-short-circuiting & operator in the while condition. If you short-circuit with a &&, you’ll have a bad value in enum2HasValue whenever enumerator1 first runs out of elements.
   However I only recommand this implementation if it’s well documented and not subjet to frequent code review, because the use of the non-short-circuiting & operator may lead to confusion or anti-optimisation (”hey, man, && is cooler, because it doesn’t evaluate more than needed !”).
   I personally wouldn’t use such a cryptic implementation : the performance should be the equivalent (if not same), and readability decreases.

6. Glenn Slaven March 19th, 2008 at 11:05 pm 6

   Ah, now I see it. They Joys of working with enumerators. I like the non-shortcuting solution, but I tend to agree with your opinion on readability over nominal performance improvement. Besides, I speed tested both ways on a massive collection & I couldn’t detect any material difference.

   Thanks for your help with this, and for your patience in getting your point through! I added in some exception handling to dispose of resources in this revision too.

7. Jurgen March 20th, 2008 at 4:21 am 7

   If you compare the IL of both versions you’ll see the non-shortcurting solution is a bit smaller (uses fewer instructions : it has less branches because ‘&&’ shortcut feature creates branches), but it shouldn’t be really faster. What takes time on big IEnumerable<T> is callvirt to MoveNext(), and both versions call MoveNext as much.

   Two more remarks and I’m done/gone (and I stop bugging you !) :
   A- You can remove the catch block. A try/finally is more than enough in that case.
   B- Don’t you want to return true also when value == compareList ? (If you do you can replace line 15 with if(value == compareList) )

   Now you’ve provided your readers with an “IsEqualTo” method, I propose an exercise for their spare time : implement “StartsWith” (easy) and “EndsWith” (ouch ! seems harder) extension methods on IEnumerable<> !

   Regards

8. Glenn Slaven March 20th, 2008 at 10:39 am 8

   This is why I like posting this stuff, I always learn so much! I’ve updated the code

9. Jurgen March 26th, 2008 at 1:28 am 9

   “Much to my surprise I found that .NET 3.5 doesn’t seem to have a native method for comparing two arrays or collections of any type.”

   Well, I’ve just discovered it does : SequenceEqual<TSource> (see http://msdn2.microsoft.com/en-us/library/bb342073.aspx) enumerates the elements and compares them.
   The main difference with IsEqualTo<TSource> is that it throws exceptions whenever one of the IEnumerable<T> is null.

   However, the names of the methods are consistent :
   - IsEqualTo compares the containers and their content if any.
   - SequenceEqual compares the content (the sequence). Therefore the containers are supposed to exists (ie not null)

   The implementation of method SequenceEqual (seen through Reflector) :
   - does not use local vars. As a result, there is one more call to MoveNext than in IsEqualTo, but the code looks straightforward in my opinion.
   - reminds me you could remove try/finally and replace that with using statements. It then behaves the same but it’s much more readable. (Haven’t seen that !.. I must still be disturbed by the fact IEnumerator<T> implements IDisposable…)