Add missing Chain#distinctBy method

[satorg]

Core:

• Adds the missing Chain#distinctBy method.
• Slightly optimizes its implementation (replaces two accesses to TreeSet via contains followed by += with just one access via add).

Tests:

• Introduces trait cats.tests.compat.ScalaVersionSpecificSyntax that implements an extension method distinctBy for any Seq-derived collection in Scala 2.12 (and does nothing for other Scala versions).
• Adds a test for Chain#distinctBy to ChainSuite.

[satorg]

Note: this distinctBy can be further optimized by returning this in cases when Chain is empty or contains a single element only. Apparently such a collection cannot contain duplicates. Something like this:

  def distinctBy[B](f: A => B)(implicit O: Order[B]): Chain[A] = {
    // Cannot just use `size` (or `length`) method here since currently they enumerate the entire collection.
    if (isEmpty || this.isInstanceOf[Chain.Singleton[_]]) this else {
      implicit val ord: Ordering[B] = O.toOrdering

      val alreadyIn = mutable.TreeSet.empty[B]
      // All the other code is the same
    }
  }

Not sure if it's really worth it though. Wdyt?

[johnynek]

Thanks!

[johnynek]

one issue: this is going to create a Chain the costs O(N) to uncons.

I wonder if the better approach would build a Vector and Wrap that. Vector seems to be the default collection we use in other places

e.g.

val bldr = Vector.newBuilder[A]
...

Wrap(bldr.result())

[satorg]

Yeah, I wonder too. I didn't really think about it, perhaps you're right. Let's try this option.

[satorg]

fixed

[satorg]

Potentially this line (and the similar one below) could be expanded into something more efficient like

bldr.result() match {
  case Vector(a) => Singleton(a)
  case seq       => Wrap(seq)
}

but... I'm not really sure it is worth it – I mean, it is unlikely that such a single call can lead to substantial (or even noticeable) performance decrease, IMO.

[johnynek]

This looks great to me!