Checking List of Lists - equality check is done by reference not by value

[lopisan]

When comparing nested lists, comparison is done by reference, not by value by value (like Enumerable.SequenceEqual does). I'd expect last two Checks in the code snippet to pass.

Code snippet:

    [Fact]
    public void CollectionTest()
    {
        var a = new List<int> {1, 2};
        var b = new List<int> {3, 4};

        // List contains references to a and b
        var list1 = new List<List<int>> {a, b};
        Check.That(list1).ContainsExactly(a, b);  // OK
        Check.That(list1).ContainsExactly(new List<List<int>> {a, b});  // OK

        // List contains new instances of lists same as a and b
        var list2 = new List<List<int>>
        {
            new List<int> {1, 2}, // new instance, same as a
            new List<int> {3, 4}  // new instance, same as b
        };
        Assert.Equal(list2, new List<List<int>> { a, b });  // XUnit assert is OK
        Check.That(list2).ContainsExactly(a, b);  // Fail
        Check.That(list2).ContainsExactly(new List<List<int>> {a, b});  // Fail
    }

PS: Added also as StackOverflow question.

[dupdob]

Thanks for providing step to reproduce the issue. I agree that NFluent current behavior is not adequate for your purpose.
It also make me realize this question has deep ramifications and will take some time to be fully covered.
Are you ok to discuss a bit about this, so I completely understand your need? Furthermore, would you be ok as well to test a beta version with the needed fix?

[lopisan]

Thanks for quick response, yes I'm ok to discuss it more as well as to test the fixed version.

[dupdob]

Thanks. I have started to work on a fix for that and I have a working version for your example. This is the easy part.
The difficult part is the API. I obviously do not want to break current behavior and I would prefer to introduce progressively.
Which brings me to my question for you:
what are you trying to achieve? it looks to me that you actually want something like:
Check.That(list2).IsEqualTo(new List<List<int>> { a, b });
Am I right? or do you actually need ContainsExactly?

[lopisan]

The example I wrote is simplified to demonstrate the problem.
I'm actually implementing a genetic algorithm, where there is a list of candidates (individuals) and each candidate is represented as a list - thus a list of lists.
A candidate is now a class subclassing a List<>, but I'll probably change it later to subclass just IEnumerable<> - so I don't like to check IsEqualTo, but really ContainsExactly.
Moreover, I'm using also a ContainsInAnyOrder for some tests and I want to have consistent naming of checks.
In some tests, I'm just using Contains, so I'd prefer to have the solution in Contains methods, not just in IsEqualTo.

I'm not sure how to implement it to API, some thoughts:

1. Check.That(list).SequenceEqual().Contains(...) - SequenceEqual() would be a flag to switch comparison style to all subsequent checks
2. Some global switch - Legacy sequence comparison that would be default (for few versions?) - Maybe I'm biased, but I'd like to have SequenceEqual as standard behaviour eventually.
3. New methods for performing a Conatins in SequenceEqual way - new names or the same names in another namespace (and I can choose by import which version I'd like)

There are cons and pros for each suggestion, and I'm not sure which I'd prefer personally, maybe 2?

PS: If you're interested, here is ContainsInAnyOrder method (It just encapsulates IsOnlyMadeOf().And.Contains()):

/// <summary>
/// Extensions to default NFluent Enumerable checks.
/// </summary>
public static class MyEnumerableCheckExtensions
{

    /// <summary>
    /// Checks that the enumerable contains all the values present in another enumerable and nothing else, in any order.
    /// </summary>
    /// <param name="check">The fluent check to be extended.</param>
    /// <param name="expectedValues">The array containing the expected values to be found.</param>
    /// <returns>A check link.</returns>
    /// <exception cref="T:NFluent.FluentCheckException">The enumerable does not contain all the expected values present in the other enumerable.</exception>
    public static IExtendableCheckLink<IEnumerable, IEnumerable> ContainsInAnyOrder<T>(this ICheck<IEnumerable> check, params T[] expectedValues)
    {
        IEnumerable possibleOneValueArray = ExtractEnumerableValueFromPossibleOneValueArray<T>(expectedValues);
        return check.ContainsInAnyOrder(possibleOneValueArray);
    }

    /// <summary>
    /// Checks that the enumerable contains all the values present in another enumerable and nothing else, in any order.
    /// </summary>
    /// <param name="check">The fluent check to be extended.</param>
    /// <param name="otherEnumerable">The enumerable containing the expected values to be found.</param>
    /// <returns>A check link.</returns>
    /// <exception cref="T:NFluent.FluentCheckException">The enumerable does not contain all the expected values present in the other enumerable.</exception>
    public static IExtendableCheckLink<IEnumerable, IEnumerable> ContainsInAnyOrder(this ICheck<IEnumerable> check, IEnumerable otherEnumerable)
    {
        return check.IsOnlyMadeOf(otherEnumerable).And.Contains(otherEnumerable);
    }

    private static IEnumerable ExtractEnumerableValueFromPossibleOneValueArray<T>(T[] expectedValues)
    {
        return !IsAOneValueArrayWithOneCollectionInside<T>(expectedValues) ? (IEnumerable)expectedValues : (object)expectedValues[0] as IEnumerable;
    }

    private static bool IsAOneValueArrayWithOneCollectionInside<T>(T[] expectedValues)
    {
        if (expectedValues != null && (long)expectedValues.Length == 1L)
            return IsAnEnumerableButNotAnEnumerableOfChars<T>(expectedValues[0]);
        return false;
    }

    private static bool IsAnEnumerableButNotAnEnumerableOfChars<T>(T element)
    {
        if ((object)element is IEnumerable)
            return !((object)element is IEnumerable<char>);
        return false;
    }


}

[dupdob]

Again, thanks for such a detailed feedback.
My own thinking led me to a similar conclusions to yours: I will probably push for SequenceEquals as default behavior.
To be a bit more specific, the check will verify if an Equals overload is available. If yes, it uses it, otherwise, it relies on the internal comparison.
That being said, it has significant consequences:

• all existing equality checks have to be upgraded
• a global setting must be provided to revert to previous behavior
• error messages have to provide correct error messages

I will push a beta where only Contains will be updated for you to check and confirm

[dupdob]

Hi @lopisan
I published a beta this morning where Enumerable equality is based on content (and not reference). In a recursive manner.
You can get it through MyGet.
You need to change the configuration of Nuget to one URL to the list of sources.
Details are here
https://www.myget.org/feed/Details/dupdobnightly
Alternatively, you can install it thanks to
Install-Package NFluent -Version 2.1.0-alpha-0136 -Source https://www.myget.org/F/dupdobnightly/api/v3/index.json

[lopisan]

Hi, sorry for late response.
I've done tests and it's working as expected.
I've checked the example I've sent in the first post as well as other more complex scenarios in my application I've needed the fix for.
I've noticed that Contains and ContainsExactly are working whereas IsOnlyMadeOf is not. It's as you've written, but I just wanted to check it.

Send me next beta if you need more beta testing.
Thanks for the fix, I'm looking forward to the next release.

[dupdob]

Hi, thanks for the feedback.
A new beta is available with full support (i.e including IsOnlyMadeOf).

Next step is to make this configurable. It will be part of V2.1

[dupdob]

To be released

[dupdob]

By the way, thanks for your input @lopisan. It looks like its your first contribution to open source project (judging by your Git profile). are you ok for me to tweet about this? do you have a twitter handle to refer to you properly?

[lopisan]

Yes, actually it is my first contribution. I was fixing/improving two opensource java testing frameworks in my previous job, but you just reminded me, that I never filled a pull request to the main project as I had planned.
Feel free to tweet about this, my twitter handle is @lopisan.