Consider renaming/unifying `prop`/`extracting`/`suspendCall`
prop isn't short for property–at least not in the Kotlin-sense. Its KFunction and lambda-accepting overloads let it operate on more than just Kotlin properties. It may be short for "property" in the general sense, but that's then confusing against the Kotlin concept and abbreviations are generally frowned upon in API signatures.
extracting is prop projected over a collection.
suspendCall is the lambda-accepting prop but suspendable.
These are all the same thing, and it would be cool if they had a unified naming convention.
I don't have any perfect naming proposals yet but I've been sitting on filing this issue for months now so I'm firing it off in the hopes others can help.
I came up with two ideas:
Center around the verb "have":
-
Assert that some user has (a) name (that) is equal to "Alice":
assertThat(someUser).has(User::name).isEqualTo("Alice") assertThat(someUser).has("name", { it.name() }).isEqualTo("Alice") -
Assert that all users each has (a) name (that) is equal to "Alice", "Bob", "Eve":
assertThat(allUsers) .eachHas(User::name) .isEqualTo(listOf("Alice", "Bob", "Eve"))Is this
eachHave? Tempted to bend grammar even if it is for consistency. -
Assert that some user has (a) DB name (that) is equal to "Alice":
assertThat(someUser).has("name", { it.dbName() }).isEqualTo("Alice")Can we pull this off, where the signature matches? Can the main one be
inlinewith some API changes maybe?
Alternatively, lean into map whose semantics are hopefully known from its available on Iterables, Flows, Rx, other languages, etc.
-
Assert that some user map(ped to a) name (that) is equal to "Alice":
assertThat(someUser).map(User::name).isEqualTo("Alice") assertThat(someUser).map("name", { it.name() }).isEqualTo("Alice") -
Assert that all users map(ped to their) name (that) is equal to "Alice", "Bob", "Eve":
assertThat(allUsers) .map(User::name) .isEqualTo(listOf("Alice", "Bob", "Eve"))Is this
eachHave? Tempted to bend grammar even if it is for consistency. -
Assert that some user map(ped to a) DB name (that) is equal to "Alice":
assertThat(someUser).map("name", { it.dbName() }).isEqualTo("Alice")
Thoughts?
(Also whoops cmd+enter too soon)
Just a comment on the proposal for using "have": a possible alternative to has could be having. The idea is that the really natural word for these is probably "with" but that is already well-used kotlin syntax so would be a very poor choice. "Having" is kind of a synonym for "with", so might read more naturally.
It would look like this:
assertThat(someUser).having(User::name).isEqualTo("Alice")
assertThat(someUser).having("name", { it.name() }).isEqualTo("Alice")
Then eachHas/eachHave would be eachHaving.
I'll try to open a PR so more code can be written against it to see what it feels like.
Yeah I chose "has" because I thought Truth used it pervasively, but it seems like they use it in quite limited fashion. The ones I use constantly are ThrowableSubject.hasMessageThat and ThrowableSubject.hasCauseThat when asserting on failures. I really thought they had more, but it doesn't seem like it.
Their use of the That-suffix is interesting. I parenthesized it out of my natural language sentence equiavlents, but I suppose it could have been included. It would be nice to choose something that didn't require both a prefix and a suffix, though, and maybe "having" gives us that.
I've just realised, it's not going to be possible to unify prop and suspendCall since prop is an instance method and suspendCall is an extension method, so giving both the same name will mean the suspend fun is shadowed.
If only you could have a suspend fun overriding a non-suspend fun (I understand why that's not possible, I'm just musing.)
prop is also an extension. I think its implementation APIs (transform and appendName) are both public so it probably could just be made inline so that it works in both contexts (like assertFailure does).
Would that be an ABI break? Might be worth doing it separately to this if it is.
We're already renaming the functions and breaking everyone. The deprecation replace with for both prop and suspendCall can both target the new, single function.
D'oh! Obviously we are!
prop isn't short for property–at least not in the Kotlin-sense. Its KFunction and lambda-accepting overloads let it operate on more than just Kotlin properties.
It originally was but became more general shortly after, I agree with the sentiment of this issue.
Yeah I chose "has" because I thought Truth used it pervasively, but it seems like they use it in quite limited fashion. The ones I use constantly are ThrowableSubject.hasMessageThat and ThrowableSubject.hasCauseThat when asserting on failures. I really thought they had more, but it doesn't seem like it.
that's an interesting connection. We do actually have a good number of 'has*' methods which may make this usage more natural.
For reference:
- assertj calls both the object and collection methods 'extracting'
- truth calls the object method 'check' and doesn't have a collection equivalent (lmk if this is wrong)
I think its implementation APIs (transform and appendName) are both public so it probably could just be made inline so that it works in both contexts (like assertFailure does).
there was a reason this wasn't done, but I forget why off the top of my head
Another note: kotlin itself doesn't unify on a name for their similar concept (let vs map), so while I do strongly agree that prop & suspendCall should be renamed I feel much less strongly that extracting should.
Linking https://github.com/willowtreeapps/assertk/issues/522#issuecomment-2018493890 as the same argument might be relevant here, a point against 'has' is that it may sound like an assertion itself instead of a middle step, ex someone might do:
assertThat(someUser).has(User::name)
not realizing it will never fail
edit: This makes me like the 'having' suggestion above a bit more, I feel like it has less of that connotation
Indeed Kotlin doesn't use the same name, although I also don't think that's a strong design decision on their part. map is basically forced on them, and let seems more designed within the let/run/with/apply set.
One important difference is that let and map operate directly on the subject whereas the functions here operate on the subject through an Assert wrapper. I think you could make an argument for having the ones here use the same root name(verb?) for the operation, but use something added like an "each" prefix or suffix when operating within an iterable.
Also there's a couple of other ways that extracting is different from prop today. It does not take a name to prepend to the failure message, and it has overloads that allow you to extract 2 or 3 values at a time. (the second on is helpful for when you want to be more specific on what you assert on but still use the contains* methods.)
ex:
assertThat(users).extracting(User::name, User::age).containsExactly(
"Alice" to 22,
"Bob" to 19
)
One thing worth exploring would be to add in addition to/replace extracting with a version of eachHaving that more directly mirrors having
ex:
assertThat(allUsers).eachHaving(User::name).isEqualTo(listOf("Alice", "Bob", "Eve"))
assertThat(allUsers).eachHaving("name") { it.name }.isEqualTo(listOf("Alice", "Bob", "Eve"))
Since I haven't seen any other suggestions given and can't think of anything better, I think we should go with the having pattern.
One thing worth exploring would be to add in addition to/replace
extractingwith a version ofeachHavingthat more directly mirrorshavingex:assertThat(allUsers).eachHaving(User::name).isEqualTo(listOf("Alice", "Bob", "Eve")) assertThat(allUsers).eachHaving("name") { it.name }.isEqualTo(listOf("Alice", "Bob", "Eve"))
I hadn't read this properly when you first posted it. I think this is a really good idea. When #524 is merged, I'll get another draft PR opened.