Added Chain#distinct and NonEmptyChain#distinct to match List#distinct and NonEmptyList#distinct
#2579
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2579 +/- ##
==========================================
+ Coverage 95.14% 95.15% +0.01%
==========================================
Files 361 361
Lines 6630 6646 +16
Branches 289 304 +15
==========================================
+ Hits 6308 6324 +16
Misses 322 322
Continue to review full report at Codecov.
|
|
|
||
| var alreadyIn = TreeSet.empty[AA] | ||
|
|
||
| foldLeft(Chain.empty[AA]) { (elementsSoFar, b) => |
There was a problem hiding this comment.
Just a question of curiosity. Why are you using TreeSet instead of pure scala's Set?
There was a problem hiding this comment.
This was done for symmetry with NonEmptyList#distinct but it's a good question. Set uses a HashSet with sets bigger than 4 elements. HashSet#contains has O(1) and TreeSet#contains has O(log(n)).
Changing Set would also remove the artificial dependency on Order. Was there a reason for this dependency when NonEmptyList#distinct was introduced?
There was a problem hiding this comment.
I wrote up a functional set implementation that should speed up distinct in the future and let it depend on Eq instead of Order. Closed issue here, was suggested to put up a PR on cats-collection instead. #2593
EDIT: Seems like this structure exists already in cats-collections, called ISet.
| /** | ||
| * Remove duplicates. Duplicates are checked using `Order[_]` instance. | ||
| */ | ||
| final def distinct[AA >: A](implicit O: Order[AA]): NonEmptyChain[AA] = { |
There was a problem hiding this comment.
I wonder. Dont you think that it would be better to to call toChain and then distinct on chain and then rebuild NonEmptyChain? It would remove duplication but I'm not sure if it will still be ok with performance.
There was a problem hiding this comment.
The other way around would be cheaper, something along the lines of NonEmptyChain.fromChain(this).fold(empty[A])(_.distinct.toChain)
Implemented and tested
Chain#distinctandNonEmptyChain#distinct.Still needs examples.