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

Support for java 8 Predicates #658

Closed
sta-szek opened this issue May 6, 2016 · 15 comments
Closed

Support for java 8 Predicates #658

sta-szek opened this issue May 6, 2016 · 15 comments
Milestone

Comments

@sta-szek
Copy link

sta-szek commented May 6, 2016

Summary

It would be nice to have support for java 8 predicates.
Assertions should allow for testing them instead of testing everyone separetly.

Real life use case / example

Good:

//given
acceptedValues = Lists.newArrayList("a","b")

//when
final Predicate<String> predicate = Predicate.create(acceptedValues);

//then
assertThat(predicate).accepts("a")
                     .accepts("b")
                     .doesNotAccept("c")
                     .accepts(Lists.newArrayList("a","b"))
                     .doesNotAccept(Lists.newArrayList("c","d"));

Not good:

//given
acceptedValues = Lists.newArrayList("a","b")

//when
final Predicate<String> predicate = Predicate.create(acceptedValues);
boolean result1 = predicate.test("a");
boolean result2 = predicate.test("b");
boolean result3 = predicate.test("c");
boolean result4 = predicate.test("d");

//then
assertThat(result1).isTrue();
assertThat(result2).isTrue();
assertThat(result3).isFalse();
assertThat(result4).isFalse();

Java 8 specific ?

YES

@joel-costigliola
Copy link
Member

Good idea, I would just use matches instead of accepts and matchAll for collection/array.

@filiphr
Copy link
Contributor

filiphr commented Jun 2, 2016

@joel-costigliola I am willing to work on this if no one is. I already have some idea about how to do it.

I just have one question, do we also need the other predicates (IntPredicate, LongPredicate, DoublePredicate).

@joel-costigliola
Copy link
Member

@filiphr that would be great ! regarding your question I would say yes (we have the same thing with Optional).

@joel-costigliola
Copy link
Member

@filiphr do you prefer assertThat(predicate).accepts("a") or assertThat(predicate).matches("a") ?
I'm having second thoughts and contradict my previous comment) as it is correct to say that a value matches a predicate but not the other way around.

@filiphr
Copy link
Contributor

filiphr commented Jun 4, 2016

@joel-costigliola I think that accepts is more suitable in this situation. You are correct.

@joel-costigliola
Copy link
Member

accepts accepted then :)

@filiphr
Copy link
Contributor

filiphr commented Jun 4, 2016

While doing the changes to accepts something else came to my mind. Regarding the error factories, I am reusing some which were already there, like ShouldMatch, and I am also reusing the Iterables#assertAllMatch is that acceptable?

@joel-costigliola
Copy link
Member

No, I think this will be confusing, if you assert that the predicate should accept a value and it does not then the problem is on the predicate not the value, I would use a message like:

Expecting given predicate to accept <value> but did not.

where given is replaced by predicate description when provided.
Feel free to suggest a better error message ...

@filiphr
Copy link
Contributor

filiphr commented Jun 4, 2016

I will modify the error messages so it is better to understand. Will probably do something in the Iterables as the logic is the same for the assertion

@joel-costigliola
Copy link
Member

👍

@jangalinski
Copy link

Nice feature. I know discussion is over, but I just have to add my two c:

I am used to the FunctionalInterfaces, so "accept" to automatically indicates a Consumer. Working with predicates I would expect "tests" (and doesNotTest).

@joel-costigliola
Copy link
Member

well if you write assertThat(predicate).test("a") it is not clear that it means assertThat(predicate).testToTrue("a") as predicate.test("a") can return true or false.

assertThat(predicate).evaluateToTrue("a") would be closer to the java api wording but a little bit too long to my taste.

@sta-szek
Copy link
Author

or according to predicate's 'test' method it could be:
assertThat(predicate).passes("a")
assertThat(predicate).passesOn("a")
assertThat(predicate).willPass("a")
assertThat(predicate).isPassing("a") ?

@joel-costigliola
Copy link
Member

I kind of like assertThat(predicate).isTrueFor("a") but I still have a slight preference for accepts.

@jangalinski
Copy link

+1 for "isTrueFor"

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
None yet
Projects
None yet
Development

No branches or pull requests

4 participants