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

Specify multiple single value extractors in flatExtracting #644

Closed
joel-costigliola opened this issue Apr 15, 2016 · 7 comments
Closed

Specify multiple single value extractors in flatExtracting #644

joel-costigliola opened this issue Apr 15, 2016 · 7 comments
Assignees
Labels
type: improvement A general improvement
Milestone

Comments

@joel-costigliola
Copy link
Member

For the time being, to extract multiple values and combine them before performing assertions we have to do something like this:

// ch type is TolkienCharacter
assertThat(fellowshipOfTheRing).flatExtracting(ch -> asList(input.getName(), ch.getAge()))
                               .contains(33 ,"Frodo", "Legolas", 1000);

It would be nice to simply specify the list of extractors (Java 8) :

assertThat(fellowshipOfTheRing).flatExtracting(TolkienCharacter::getName, 
                                               TolkienCharacter::getAge))
                               .contains(33 ,"Frodo", "Legolas", 1000);

or with properties names (Java 7) :

assertThat(fellowshipOfTheRing).flatExtracting("name", "age"))
                               .contains(33 ,"Frodo", "Legolas", 1000);
@PascalSchumacher
Copy link
Member

I already have working prototype for the multiple extractor function method. I will clean it up and submit a pull requests.

@PascalSchumacher
Copy link
Member

PascalSchumacher commented Apr 17, 2016

Just noticed that the prototype is not working. It was expecting all collections to contain objects of the same type.

When I tried to make it work for untyped objects, there is a conflict between the two method signatures:

public <V> ListAssert<V> flatExtracting(Extractor<? super T, ? extends Collection<V>> extractor)

public ListAssert<Object> flatExtracting(Function<Object, ? extends Collection<Object>>... extractors)

so it does not seem to be possible to make this work (unless I'm missing something).

@joel-costigliola
Copy link
Member Author

joel-costigliola commented Apr 18, 2016

I think what we want is :

public ListAssert<Object> flatExtracting(Function<Object, Object>... extractors)
// or
public ListAssert<Object> flatExtracting(Extractor<Object, Object>... extractors)

and not:

public ListAssert<Object> flatExtracting(Function<Object, ? extends Collection<Object>>... extractors)

the reason being that each extractor extracts one value, combine them in a list and flatten it.
conceptually it is converting a list of single value extractors to one multi value extractor, like allAsList in guava's future.

This will solve the compilation problem I think.

As a side note, I'm not sure whether we would need to use wildcard or not, e.g:

public ListAssert<Object> flatExtracting(Extractor<? super Object, ? extends Object>... extractors)

@PascalSchumacher
Copy link
Member

PascalSchumacher commented Apr 18, 2016

I do not understand. If we want to extract a single value why use flatExtracting? The existing method public <V> ListAssert<V> flatExtracting(Extractor<? super T, ? extends Collection<V>> extractor) does not extract single values. For extracting single values is the extracting method. Or I'm I missing something?

Anyway:

public ListAssert<Object> flatExtracting(Function<Object, Object>... extractors)
public ListAssert<Object> flatExtracting(Extractor<Object, Object>... extractors)
public ListAssert<Object> flatExtracting(Extractor<? super Object, ? extends Object>... extractors)

each cause a compile time conflict with the existing method:

public <V> ListAssert<V> flatExtracting(Extractor<? super T, ? extends Collection<V>> extractor)

@joel-costigliola
Copy link
Member Author

joel-costigliola commented Apr 19, 2016

We want to extract multiple single values as in:

assertThat(fellowshipOfTheRing).flatExtracting(TolkienCharacter::getName, 
                                               TolkienCharacter::getAge))
                               .contains(33 ,"Frodo", "Legolas", 1000);

We combine them in a list so that our users don't have to do it manually as in:

// ch type is TolkienCharacter
assertThat(fellowshipOfTheRing).flatExtracting(ch -> asList(input.getName(), ch.getAge()))
                               .contains(33 ,"Frodo", "Legolas", 1000);

@PascalSchumacher does it make sense to you ?

@PascalSchumacher
Copy link
Member

Sure, thanks for explaining. :)

@joel-costigliola joel-costigliola self-assigned this Apr 26, 2016
@joel-costigliola joel-costigliola modified the milestones: 3.5.0, 2.5.0 May 31, 2016
joel-costigliola added a commit that referenced this issue Jun 3, 2016
@joel-costigliola
Copy link
Member Author

Done for 2.5.0 by specifying extractors by name and 3.5.0 by specifying extractors as lambdas.

joel-costigliola added a commit that referenced this issue Jun 8, 2016
fiery-phoenix pushed a commit to fiery-phoenix/assertj-core that referenced this issue Mar 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: improvement A general improvement
Projects
None yet
Development

No branches or pull requests

2 participants