Make Chain Arbitraries recursively build concatenations #2430
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2430 +/- ##
==========================================
+ Coverage 95.21% 95.27% +0.06%
==========================================
Files 351 351
Lines 6369 6350 -19
Branches 286 274 -12
==========================================
- Hits 6064 6050 -14
+ Misses 305 300 -5
Continue to review full report at Codecov.
|
| rights.trimEnd(1) | ||
| } | ||
| // Empty is only top level, it is never internal to an Append | ||
| if (rights.nonEmpty) throw new IllegalStateException(s"found internal Empty in $this") |
There was a problem hiding this comment.
should we just remove this since 1. we believe this to be true, 2. we have laws to catch this. By removing entirely we improve code coverage and we possibly slightly improve performance.
| @@ -461,8 +455,8 @@ object Chain extends ChainInstances { | |||
| rights.clear() | |||
| currentIterator = seq.iterator | |||
| currentIterator.next | |||
| case Empty => | |||
| go // This shouldn't happen | |||
| case null | Empty => | |||
There was a problem hiding this comment.
without the null here we had a match error previously on empty.
There was a problem hiding this comment.
Huh, I didn't even know you could pattern match on null 😬
| @@ -505,8 +499,8 @@ object Chain extends ChainInstances { | |||
| lefts.clear() | |||
| currentIterator = seq.reverseIterator | |||
| currentIterator.next | |||
| case Empty => | |||
| go // This shouldn't happen | |||
| case null | Empty => | |||
There was a problem hiding this comment.
without the null here we had a match error previously on empty.
|
Ready to merge this @johnynek? |
* Make Chain Arbitraries recursively build concatenations * check some emptyness invariants * improve code coverage by leveraging invariants * test next throws the right exception * remove runtime check * make Chain iterators private
forgive me @LukaJCB I wanted a direct Arbitrary that constructs in all the ways we can (empty, single, concat or fromList).
I think this current generator should create any possible chain with some probability.