-
Notifications
You must be signed in to change notification settings - Fork 88
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
Consider unifying first-party, prop
-based helper function names (like jClass
, toStringFun
)
#522
Comments
Once there's an agreement on how to resolve #521, happy to handle this |
There is a subtle distinction there, the 'has*' methods do an assertion themselves while the non-has methods can't fail on their own. One thing I'd want to avoid is someone doing |
Perhaps "having" could help here, as it seems less like an assertion:
feels less like a complete sentence. |
I also think |
Perhaps that's a useful general principle for this kind of thing; a method named with a verb which isn't an assertion should use the continuous ("ing") form of the verb. That's almost certainly a premature abstraction, and wrong in the details, but I thought it was worth noting down. |
Hm, looking at the draft mr I'm not sure I'm convinced that adding a |
What are your primary examples of occurrences you don't like? |
From a wider context we are introducing api churn. Which in-itself isn't a bad thing but does mean that there should be a good reason for it. Let's take assertThat(exception)
.message()
.isNotNull()
.matches(Regex(...)) to assertThat(exception)
.havingMessage()
.isNotNull()
.matches(Regex(...)) The first one is shorter and reads slightly better. Like it's not a huge difference but if we are going to change things it should be a more obvious win no? |
note: this was partially spurred on by #521 (comment), which also points out than androidx has started using a similar pattern in their truth extensions |
I actually think the It produces "assert (that) exception having message is not null matches regex". It also means that you can type ".having" to see all built-in transformations as well as complete to the lambda variation to do custom transforms. |
FWIW, I agree with Jake. I think the consistency of all helper functions starting with "having" helps distinguish them. |
Since I can send a PR if so. |
There's definitely an inconsistency with these functions today. |
The current naming is mostly just the property name as the function name. There's
kClass()
, collectionsize()
s, exceptionmessage()
.This strategy doesn't work for
toString()
orhashCode()
. They get "Fun" suffixes astoStringFun()
andhashCodeFun()
, respectively.This ties in somewhat with #521, and it actually leans me towards that issue choosing "have" so that these could be "has"-prefixed.
The text was updated successfully, but these errors were encountered: