Add collection filtering assertions analagous to filterIsInstance #546
Conversation
jzbrooks
force-pushed
the
instance-of
branch
from
669f726 to
45ec062
Compare
| - added `doesNotExist` assertions to `Path`. | ||
|
|
||
| ### Added | ||
| - Added `doesNotExist` assertions to `Path` |
There was a problem hiding this comment.
I moved this note into the added section for consistency
| */ | ||
| inline fun <reified T> Assert<Iterable<*>>.containsInstanceOf(): Assert<List<T>> { | ||
| return transform("contains subtype of ${T::class}") { actual -> | ||
| actual.filterIsInstance<T>().also { |
There was a problem hiding this comment.
I would use any here so that it returns true as soon as a single instance is found rather that needing to perform a full iteration and allocate a new list.
There was a problem hiding this comment.
The sub-list allocation is necessary to provide a way to continue making assertions against the sublist. No?
There was a problem hiding this comment.
Ah, yeah, I missed the new list was propagated as a return value. I'm a bit hesitant about the API shape using a verb "contains" while the return value is a full filtering.
There was a problem hiding this comment.
Yeah, agreed. I noticed the having discussion after I pushed this. Would havingInstancesOf be better? Maybe the negation would be doesNotHaveInstancesOf?
There was a problem hiding this comment.
I guess I'm a bit torn on whether this makes sense as a dedicated operator since it combines a transform with an assertion in a way that I don't think has precedent from other operators. We'll see what the actual maintainers think, though.
There was a problem hiding this comment.
I'd argue there's precedent with isNotNull. It's a transform and an assertion.
There was a problem hiding this comment.
But it's atomic in nature. You cannot break it down into discrete steps.
Whereas this one is very clearly a filterIsInstance transformation and then an isNotEmpty assertion.
There was a problem hiding this comment.
Fair. I'd find a shorthand for the transformation basically just as helpful.
There was a problem hiding this comment.
A transformation without assertion might set a (good) precedent for other collection operators.
@evant, could you weigh in when you have some time?
| * ``` | ||
| */ | ||
| inline fun <reified T> Assert<Iterable<*>>.doesNotContainInstanceOf() = given { actual -> | ||
| if (actual.filterIsInstance<T>().isNotEmpty()) expected("to contain no instances of ${T::class} but was $actual") |
jzbrooks
changed the title
|
#554 might be relevant |
| * ``` | ||
| */ | ||
| inline fun <reified T> Assert<Iterable<*>>.containsInstanceOf(): Assert<List<T>> { | ||
| return transform("contains subtype of ${T::class}") { actual -> |
There was a problem hiding this comment.
This message should probably just be contains instance of ${T::class} per Liskov.
|
Along the lines of the |
jzbrooks
force-pushed
the
instance-of
branch
from
fd0c651 to
0823654
Compare
jzbrooks
force-pushed
the
instance-of
branch
from
0823654 to
d2e7108
Compare
|
Alternatively |
This would be pretty handy when dealing with collections of sealed typed objects. I'd use it a lot!