-
Notifications
You must be signed in to change notification settings - Fork 26
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
Fix documentation/name for AbstractMapAssert#containsOnlyKeys
#23
Comments
AbstractMapAssert.html#containsOnlyKeys
AbstractMapAssert#containsOnlyKeys
Hi @C-Otto,
Could you write the text you would you like to see in the javadoc? |
To me the word "only" in "X ... only Y" implies a restriction on X so that Y must be satisfied. As such, the condition is satisfied, as long as every item of the set X conforms to whatever Y expresses. This also means that "X ... only Y" automatically is satisfied for empty X, no matter the Y. I'm not a native speaker, but several translations to my native language lead to the same thoughts. |
But the condition is not on every item, it's on the collection. |
I totally get what the code is doing, and in fact it is doing what I need. I just don't like the name and the documentation, as they don't match the code (in my understanding). I also see that you have a different understanding. I'd just like to avoid confusion by making the description easier to understand. I might just be plain wrong, that's for you to decide. An empty collection does not contain anything, that's correct. I also see that the invocation you mentioned should fail. However, the "only" in the method name still leads me to the thought that it should pass! "The keys that are contained in the actual map (namely none) satisfiy any criterion, especially the one that they are key1 or key2 or key3, and nothing else. As such, the check should be true." Maybe it helps if we don't talk about the empy map/set/...?
I'd say this is OK, as every single key in the actual map ("a") is either "a" or "b". |
Hi @C-Otto, based on your last update, I wanted to react on:
The behavior described would fit an assertion named like Would you consider this issue still valid? |
Yes. My expectation based on my background in math/logic and my non-native English skills:
For me, "X containsOnly p" translates to "for all x in X it holds that p(x) is true". "for all" and "there exists" are very fundamental principles in math/logic, and for me AssertJ does not stick to the semantics I'm used to. |
I'm sorry, on some points I struggle to follow your reasoning. I understand that you refer as I don't understand what If the assertion were named |
I use Yes, |
According to the method name and it's documentation I expect
assertThat(emptyMap()).containsOnlyKeys("x")
to NOT fail.There's one example in the Javadoc that helps me understand the real semantics. However, I'd like to see that also in the text above, and ideally in the method name.
Related: https://math.stackexchange.com/questions/202452/why-is-predicate-all-as-in-allset-true-if-the-set-is-empty
See assertj/assertj#200
The text was updated successfully, but these errors were encountered: