Skip to content

Conversation

@vanya-lebedev
Copy link
Contributor

@vanya-lebedev vanya-lebedev commented Feb 27, 2021

Introduce a version of the SelfReferencingCollectionAssertions.SatisfyRespecitvely method which is not sensitive to the order of elements and accepts predicate expressions.

Example of an assertion:

plannedOrders.Should().Satisfy(
    order => order.Type == OrderType.Purchase && order.Quantity == 5 && order.Date == tomorrow,
    order => order.Type == OrderType.Transfer && order.SourceLocation == distributionCenter && order.Quantity == 3 && order.Date == today,
    order => order.Type == OrderType.Transfer && order.SourceLocation == factory && order.Quantity == 5 && order.Date == today.AddDays(10)
);

Example of a failure message:

Expected collection to satisfy all predicates because we want to test formatting (args), but:
    
    The following predicates did not have matching elements:
    
    (_.Text == "two") AndAlso (_.Number == 2)
    
    The following elements did not match any predicate:
    
    FluentAssertions.Specs.Collections.GenericCollectionAssertionsSpecs+SomeClass
    {
       Number = 1
       Text = "one"
    }
    
    FluentAssertions.Specs.Collections.GenericCollectionAssertionsSpecs+SomeClass
    {
       Number = 3
       Text = "two"
    }
    
    FluentAssertions.Specs.Collections.GenericCollectionAssertionsSpecs+SomeClass
    {
       Number = 3
       Text = "three"
    }

The discussion about this API is tracked here:
#1480

To enable the necessary functionality three major changes were introduced:

  • The Satisfy API was added to the SelfReferencingCollectionAssertions class.
  • The MaximumMatchingSolver class was added to encapsulate the simplified version of the Ford-Fulkerson algorithm to find matching between the asserted predicates and the elements of the collection.
  • The PredicateLambdaExpressionValueFormatter class was introduced to produce more human-readable descriptions of predicates in assertion failure messages.

IMPORTANT

  • The code complies with the Coding Guidelines for C#.
  • The changes are covered by a new or existing set of unit tests which follow the Arrange-Act-Assert syntax such as is used in this example.
  • If the contribution adds a feature or fixes a bug, please update the release notes, which are published on the website.
  • If the contribution changes the public API the changes needs to be included by running AcceptApiChanges.ps1/AcceptApiChanges.sh.
  • If the contribution affects the documentation, please include your changes in this pull request so the documentation will appear on the website.

@vanya-lebedev vanya-lebedev marked this pull request as ready for review February 27, 2021 19:33
@dennisdoomen
Copy link
Member

That's an impressive piece of work. Thanks for that.

Copy link
Member

@jnyrup jnyrup left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really nice work!
Only minor comments from here.

@vanya-lebedev vanya-lebedev requested a review from jnyrup February 28, 2021 19:12
@vanya-lebedev
Copy link
Contributor Author

Dennis, Jonas, thanks a lot for your support!

Copy link
Member

@dennisdoomen dennisdoomen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

**Comments

  • 👍 Impressive work. You've kind of changed my mind about the importance of algorithms,
  • 🔧 I think it reads better if you replace the term NotMatched with Unmatched everywhere.
  • 🔧 Since the namespace is already called MaximumMatching, I would repeat that word on every class and method.
  • 🔧 I think the code will be much easier to understand if you would wrap the predicate and its index in a first-class class such as Predicate, and then have it override Equals. This will reduce the amount of fiddling around with indexes and using those to find the actual predicate. And this probably also applies to the element collection and its indexes.

Legend for the emojis
• ❓ I have a concrete question. There is nothing here to be changed yet.
• ❌ Here is an actual problem. Please change this according to my suggestion. It might be a bug or a strict code style fix.
• 🔧 Here is an idea/suggestion. I think it would improve things, but feel free to disagree :)
• 🙃 This is a nitpick. Feel free to change of leave it.
• 🤔 This might be wrong. Let’s talk about it, maybe we can come up with a better solution
• 🤡 Here is a problem independent from your work. It would be nice if you would be willing to include it

@jnyrup
Copy link
Member

jnyrup commented Mar 2, 2021

* 🔧 I think the code will be much easier to understand if you would wrap the predicate and its index in a first-class class such as `Predicate`, and then have it override `Equals`. This will reduce the amount of fiddling around with indexes and using those to find the actual predicate. And this probably also applies to the element collection and its indexes.

A middle way could maybe be to make two small structs to wrap the raw ints making it more visible what they represent.

struct PredicateIndex : IEquatable<PredicateIndex>
{
    private int value;
    ...
}

struct ElementIndex : IEquatable<ElementIndex>
{
    private int value;
    ...
}

Then

private readonly Dictionary<int, int> matchedElementIndexesByPredicateIndex;

could be

private readonly Dictionary<PredicateIndex, ElementIndex> matchedElementIndexes;

@vanya-lebedev
Copy link
Contributor Author

Pardon. I didn't realize that I was pushing iterations without squashing my local commits. I'm used to working with Azure Dev Ops rather than Git Hub and it does squashing automatically.

@dennisdoomen dennisdoomen changed the title Ilebedev/exactly contain Adds SelfReferencingCollectionAssertions.SatisfyRespecitvely method which is not sensitive to the order of elements and accepts predicate expressions. Mar 6, 2021
@jnyrup jnyrup changed the title Adds SelfReferencingCollectionAssertions.SatisfyRespecitvely method which is not sensitive to the order of elements and accepts predicate expressions. Adds SelfReferencingCollectionAssertions.Satisfy method which is not sensitive to the order of elements and accepts predicate expressions. Mar 6, 2021
@jnyrup jnyrup merged commit aa2672a into fluentassertions:develop Mar 6, 2021
@jnyrup jnyrup linked an issue Mar 7, 2021 that may be closed by this pull request
@vanya-lebedev vanya-lebedev deleted the ilebedev/ExactlyContain branch October 2, 2021 17:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Idea for new collection assertion

3 participants