Skip to content

Conversation

@veleek
Copy link
Contributor

@veleek veleek commented Nov 6, 2019

Adding support for HaveSameCount and NotHaveSameCount for dictionaries. This was almost directly cribbed from the existing CollectionAssertions (including test cases).

Resolves #1176

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 affects the documentation, please include your changes to documentation.md in this pull request so the documentation will appear on the website.

@veleek veleek changed the title [WIP] Add Have/NotHaveSameCount for Dictionaries Add Have/NotHaveSameCount for Dictionaries Nov 6, 2019
{
Guard.ThrowIfArgumentIsNull(otherCollection, nameof(otherCollection), "Cannot compare dictionary count against a <null> collection.");

if (ReferenceEquals(Subject, null))
Copy link
Member

Choose a reason for hiding this comment

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

Considering using is null.

Copy link
Member

Choose a reason for hiding this comment

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

The reason is null isn't used in CollectionAssertions.HaveSameCount is because TSubject is not constrained to reference types and is null can only be used on reference types.

Copy link
Member

Choose a reason for hiding this comment

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

SonarQube keeps telling me (in another project) to compare to default(T) ;-)

@veleek
Copy link
Contributor Author

veleek commented Nov 8, 2019

If there are no other concerns, what's the process for getting this merged?

@dennisdoomen dennisdoomen requested a review from jnyrup November 9, 2019 06:27
@jnyrup
Copy link
Member

jnyrup commented Nov 9, 2019

@veleek Could I ask you to change the two ReferenceEquals(Subject, null) to Subject is null.
That will align with the rest of the project where using (generically constrained) reference types?
Then I'll be happy to push to the merge button 👍

Use `is null` instead of `ReferenceEquals(..., null)`.
@jnyrup jnyrup merged commit c477f8b into fluentassertions:master Nov 11, 2019
@jnyrup
Copy link
Member

jnyrup commented Nov 11, 2019

@veleek Thanks for the contribution!

@dennisdoomen
Copy link
Member

Thanks for the contribution!

Indeed. Another nice little gem.

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.

GenericDictionaryAssertions doesn't have many of the GenericCollectionAssertions methods

3 participants