Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[API Suggestion]: Set functionality for DataRowCollections #1896

Closed
logiclrd opened this issue Apr 16, 2022 · 15 comments
Closed

[API Suggestion]: Set functionality for DataRowCollections #1896

logiclrd opened this issue Apr 16, 2022 · 15 comments
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation

Comments

@logiclrd
Copy link
Contributor

logiclrd commented Apr 16, 2022

The generic collection assertions include set functionality:

  • BeSubsetOf / NotBeSubsetOf
  • IntersectWith / NotIntersectWith

This functionality used to be available for assertions on DataRowCollection objects, before support for non-generic collection assertions was removed. This removal unintentionally eliminated assertions on System.Data collections completely, as it was implemented implicitly via the collections' IEnumerable support. System.Data precedes generics in the language, and has never been updated to use IEnumerable<T>. Recently, in #1812, the ability to do these assertions on System.Data collections was reinstated, but the set support was set off for separate implementation.

A PR has been created that adds the set support for DataRowCollection as well: #1894. It has been requested that an issue be created to discuss these API changes independent of the implementation, and that's what this is.

public static class DataRowCollectionAssertionExtensions
{
    public static AndConstraint<GenericCollectionAssertions<DataRow>> BeSubsetOf(this GenericCollectionAssertions<DataRow> assertion, IEnumerable<DataRow> expectedSuperset, string because = "", params object[] becauseArgs) { }
    public static AndConstraint<GenericCollectionAssertions<DataRow>> BeSubsetOf(this GenericCollectionAssertions<DataRow> assertion, DataRowCollection expectedSuperset, string because = "", params object[] becauseArgs) { }
    public static AndConstraint<GenericCollectionAssertions<DataRow>> BeSubsetOf(this GenericCollectionAssertions<DataRow> assertion, IEnumerable<DataRow> expectedSuperset, Func<EquivalencyAssertionOptions<DataRow>, EquivalencyAssertionOptions<DataRow>> config, string because = "", params object[] becauseArgs) { }
    public static AndConstraint<GenericCollectionAssertions<DataRow>> IntersectWith(this GenericCollectionAssertions<DataRow> assertion, IEnumerable<DataRow> otherRows, string because = "", params object[] becauseArgs) { }
    public static AndConstraint<GenericCollectionAssertions<DataRow>> IntersectWith(this GenericCollectionAssertions<DataRow> assertion, DataRowCollection otherCollection, string because = "", params object[] becauseArgs) { }
    public static AndConstraint<GenericCollectionAssertions<DataRow>> IntersectWith(this GenericCollectionAssertions<DataRow> assertion, IEnumerable<DataRow> otherRows, Func<EquivalencyAssertionOptions<DataRow>, EquivalencyAssertionOptions<DataRow>> config, string because = "", params object[] becauseArgs) { }
    public static AndConstraint<GenericCollectionAssertions<DataRow>> NotBeSubsetOf(this GenericCollectionAssertions<DataRow> assertion, IEnumerable<DataRow> unexpectedSuperset, string because = "", params object[] becauseArgs) { }
    public static AndConstraint<GenericCollectionAssertions<DataRow>> NotBeSubsetOf(this GenericCollectionAssertions<DataRow> assertion, DataRowCollection unexpectedSuperset, string because = "", params object[] becauseArgs) { }
    public static AndConstraint<GenericCollectionAssertions<DataRow>> NotBeSubsetOf(this GenericCollectionAssertions<DataRow> assertion, IEnumerable<DataRow> unexpectedSuperset, Func<EquivalencyAssertionOptions<DataRow>, EquivalencyAssertionOptions<DataRow>> config, string because = "", params object[] becauseArgs) { }
    public static AndConstraint<GenericCollectionAssertions<DataRow>> NotIntersectWith(this GenericCollectionAssertions<DataRow> assertion, IEnumerable<DataRow> otherRows, string because = "", params object[] becauseArgs) { }
    public static AndConstraint<GenericCollectionAssertions<DataRow>> NotIntersectWith(this GenericCollectionAssertions<DataRow> assertion, DataRowCollection otherCollection, string because = "", params object[] becauseArgs) { }
    public static AndConstraint<GenericCollectionAssertions<DataRow>> NotIntersectWith(this GenericCollectionAssertions<DataRow> assertion, IEnumerable<DataRow> otherRows, Func<EquivalencyAssertionOptions<DataRow>, EquivalencyAssertionOptions<DataRow>> config, string because = "", params object[] becauseArgs) { }
}
@jnyrup jnyrup added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Apr 16, 2022
@jnyrup
Copy link
Member

jnyrup commented Apr 17, 2022

Support for set operations on System.Data was restored with #1812.
What is proposed here is a completely new feature that combines set operations and equivalency comparisons.

@logiclrd
Copy link
Contributor Author

logiclrd commented Apr 17, 2022

So in your mind, this is not logically the same thing as BeSubsetOf, IntersectWith, NotBeSubsetOf and NotIntersectWith as provided by GenericCollectionAssertions? I know it's not literally the same thing, but it fills the same niche, given that literal reference comparisons between System.Data objects isn't going to be terribly useful. You can never have two different tables literally contain the actual same DataRow object.

@jnyrup
Copy link
Member

jnyrup commented Apr 18, 2022

I get that because System.Data types do not override bool object.Equals(object) the existing set operations will do reference comparisons, which isn't useful for your case and hence you propose more powerful set operations tailored for System.Data that use equivalency comparison.

@dennisdoomen
Copy link
Member

If somebody works a lot with DataSet and its contained elements, I kind of get why you want to do what your PR offers. But I most definitely would not expose the equivalency options. Just make sensible assumptions about how to functionally compare data rows and columns.

@jnyrup
Copy link
Member

jnyrup commented Apr 24, 2022

@logiclrd For my understanding, is this proposal something you would find useful, as in actually use it, or is it more of hypothetical usefulness?

@logiclrd
Copy link
Contributor Author

I don't currently have any tests that would use this functionality -- though that is at least partly because I didn't have a way to write such tests. There is something of a chicken-and-egg problem, it seems to me -- if the functionality is there then it has some likeliness of being consumed, but if it's not there, then it isn't terribly likely that people will write code to do the comparison by hand.

@dennisdoomen
Copy link
Member

So unless anybody really needs this (and is willing to build it), I don't think I want to invest time in it. What do you think @jnyrup

@logiclrd
Copy link
Contributor Author

logiclrd commented Aug 6, 2022

Okay -- since this consists entirely of extension methods, it seems eminently possible to put these into their own package off the main FluentAssertions source code. So, I could transplant these into a FluentAssertions.DataSetExtensions package or someset. Does that seem reasonable??

@jnyrup
Copy link
Member

jnyrup commented Aug 7, 2022

I don't think this should be part of the core library.
It's always perfectly reasonable to implement this in your own library - it's one of our design goals that the library should be easy to extend.

FYI, you might run into problems similar to #1486.

@jnyrup jnyrup closed this as completed Aug 7, 2022
@logiclrd
Copy link
Contributor Author

logiclrd commented Aug 7, 2022

Mm, interesting -- that's only a problem when elements in one set can be equivalent to more than one element in the other set in different ways, if I'm understanding correctly?

@logiclrd
Copy link
Contributor Author

logiclrd commented Aug 7, 2022

The current prototype implementation of the DataRowCollection set functionality doesn't do any ordering checks. Is there a way to query EquivalencyAssertionOptions<TExpectation> from the outside to find out whether the caller has supplied options configured with or without a strict ordering requirement?

@jnyrup
Copy link
Member

jnyrup commented Aug 7, 2022

Mm, interesting -- that's only a problem when elements in one set can be equivalent to more than one element in the other set in different ways, if I'm understanding correctly?

Correct.

Is there a way to query EquivalencyAssertionOptions<TExpectation> from the outside to find out whether the caller has supplied options configured with or without a strict ordering requirement?

Have a look at OrderingRuleCollection.IsOrderingStrictFor(IObjectInfo objectInfo)

@logiclrd
Copy link
Contributor Author

logiclrd commented Aug 8, 2022

Ah, okay, and I can access that through the IEquivalencyAssertionOptions interface, cool cool. Thanks :-)

@logiclrd
Copy link
Contributor Author

logiclrd commented Aug 8, 2022

Oh, problem though -- IObjectInfo implementations are internal. Not exactly sure how to use it either, but them being internal is the bigger problem. Or do I just have to make my own implementation??

@logiclrd
Copy link
Contributor Author

logiclrd commented Aug 8, 2022

I guess that's the answer? Just make my own copy of the basic ObjectInfo implementation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation
Projects
None yet
Development

No branches or pull requests

3 participants