StephenCleary / Comparers

The last comparison library you'll ever need!
MIT License
427 stars 33 forks source link

Order insignificant sequence equality comparison #1

Closed franklin-ross closed 7 years ago

franklin-ross commented 10 years ago

Hey there,

I've got an extension function which has been kicking around in my LinqPad extensions for while which is basically your .EquateSequence() function, except that it allows the significance of element order to be specified. It takes duplicate elements in each sequence into account when order is not significant, otherwise it just uses System.Linq.Enumerable.SequenceEqual() for order significant equality.

I'd be happy to give you the code, or submit a pull request or something if you'd like to include it in your library?

Cheers, Frank.

StephenCleary commented 10 years ago

That sounds good; go ahead and put it in a gist or something and I'll take a look.

What do you think it should be named?

franklin-ross commented 10 years ago

No worries, here's a link to the code: https://gist.github.com/alpha-cast/e57a2e834f1c8ddd6240#file-order-insignificant-sequence-comparer And here are some tests in LinqPad: https://gist.github.com/alpha-cast/51d4ffe268c928ee55a9#file-tests-for-order-insignificant-sequence-compare

Now that I look at it, I don't believe the code depends on your library at all, but it does play nicely with it, and I think it'd be a nice addition.

The file includes an extension function which overrides your own, I think it's a good name:

   EquateSequence<TElement>(
           this IEqualityComparer<TElement> sourceComparer,
           ElementOrder orderSignificance = ElementOrder.Significant,
           bool allowNulls = false)

The null element handling was a little tricky, but I'm pretty sure I've found a way to support it without having to interrogate the element comparer.

franklin-ross commented 10 years ago

Actually, I tried to use your sequence equate function like this elementComparer.SequenceEquate().Equals(sequenceWithNulls, anotherSequenceWithNulls) to do my order significant compare, but it exploded in my face and took LinqPad down. That usually only happens for those nasty uncatchable .NET exceptions.

I didn't really look into it, so it's possible I was just doing something stupid. I'm back to using Enumerable.SequenceEqual(a, b, comparer) which seems to work OK for null elements too.

StephenCleary commented 10 years ago

I'm unable to repro this with the following test:

[TestMethod]
public void SimpleTest()
{
    var elementComparer = EqualityCompare<int?>.Default();
    var result = elementComparer.EquateSequence().Equals(new int?[] { null }, new int?[] { null });
    Assert.IsTrue(result);
}
franklin-ross commented 10 years ago

You mean for the crash? Yeah I'm not sure what the problem there was. Let's just assume it was me doing something dumb, I didn't spend any time trying to work out why that was happening, but I think you library is pretty solid.

Alright, I'm just heading out to a bucks party now. If you like I'll have a look tomorrow and see whether I can reimplement the file in terms of your library, which might simplify a couple of things and make it integrate more easily. On 25/10/2014 7:47 am, "Stephen Cleary" notifications@github.com wrote:

I'm unable to repro this with the following test:

[TestMethod] public void SimpleTest() { var elementComparer = EqualityCompare<int?>.Default(); var result = elementComparer.EquateSequence().Equals(new int?[] { null }, new int?[] { null }); Assert.IsTrue(result); }

— Reply to this email directly or view it on GitHub https://github.com/StephenCleary/Comparers/issues/1#issuecomment-60446958 .

franklin-ross commented 10 years ago

Sorry to leave this so long. Still planning on getting to it at some point, but life is hectic at the moment!

StephenCleary commented 7 years ago

Updated links due to user rename: https://gist.github.com/franklin-ross/e57a2e834f1c8ddd6240#file-order-insignificant-sequence-comparer https://gist.github.com/franklin-ross/51d4ffe268c928ee55a9#file-tests-for-order-insignificant-sequence-compare

StephenCleary commented 7 years ago

It seems to me that populating collections would be surprising behavior for a comparer. I think this would be best left as an explicit add-on, rather than part of the comparer core.