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

Feature/3165 recursive comparison assert #3179

Open
wants to merge 3 commits into
base: 3.x
Choose a base branch
from

Conversation

amodolo
Copy link
Contributor

@amodolo amodolo commented Sep 15, 2023

Check List:

Add RecursiveComparisonAssert.withEqualsForTypes and RecursiveComparisonAssert.withComparatorForTypes methods to compare different type objects in a type-safe way. For the latter, I've added a BiComparator functional interface to avoid the declaration of a Comparator<Object> that requires a forced cast of the instances to be compared.

Furthermore, to fix the type clash generated by this kind of registration

assertThat(actual).usingRecursiveComparison()
                  .withComparatorForTypes(comparator1, String.class, LocalTime.class)
                  .withComparatorForTypes(comparator2, String.class, LocalDate.class)

I've modified the internal TypeHolder data structure from
protected final Map<Class<?>, T> typeHolder;
to
protected final Map<Pair<Class<?>, Class<?>>, T> typeHolder;
This allows us to associate a Comparator to a type's pair ([String, LocalTime] -> comparator1).

Retro compatibility is maintained; in the following scenario

assertThat(actual).usingRecursiveComparison()
                  .withComparatorForType(comparator1, String.class)

The typeHolder will contain this entry ([String, null] -> comparator1).

The only method that has changed the signature is TypeHolder.entityByTypes from
public Stream<Entry<Class<?>, T> entityByTypes()
to
public Stream<Entry<Pair<Class<?>, Class<?>>, T>> entityByTypes()
that's why this cannot be considered as a safe change...let's talk about this

@joel-costigliola joel-costigliola added the theme: recursive comparison An issue related to the recursive comparison label Sep 16, 2023
@joel-costigliola
Copy link
Member

Thanks @amodolo, I will review your PR in the next few weeks.

Copy link
Member

@joel-costigliola joel-costigliola left a comment

Choose a reason for hiding this comment

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

@amodolo thanks for the good quality of the code, I have finished the first round of reviews mostly on the production code, I have some questions that can impact the PR quite a bit, so I prefer we tackle them before I continue on the review since this could change the PR significantly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme: recursive comparison An issue related to the recursive comparison
Projects
None yet
2 participants