Prepend a Vector to a NonEmptyVector

[ssanj]

I needed to implement this recently and was surprised it didn't already exist.

Thank you for contributing to Cats!

This is a kind reminder to run sbt +prePR and commit the changed files, if any, before submitting.

[travisbrown]

👍, thanks, I think this is a reasonable thing to add, especially since we have something similar for other non-empty collection types (e.g. prependLazyList and prependChain, although not prependList for some reason).

I do think it'd be better to implement it like this:

new NonEmptyVector(vector +: toVector)

This is shorter, clearer, more consistent with similar definitions like prepend and concat, and avoids creating an Option and a couple of function values.

[travisbrown]

What do you think of naming this prependVector for consistency, on the model of the existing prependLazyList and prependChain?

[ssanj]

Sounds good. Happy to rename it so it's consistent with similar constructs. 👍

[travisbrown]

Thanks much!

[ssanj]

Only a sightly related note do you know why the internal implementation of NonEmptyVector is unsafe:

final class NonEmptyVector[+A] private (val toVector: Vector[A])

and we have implementations like this:

def head: A = toVector.head

which are also unsafe?

I presume clients of the API can only create an instance of this class through its typesafe companion functions. But this would still open up the possibility to create a NonEmptyVector incorrectly from within the class itself.

NonEmptyList does not seem to be implemented similarly and is typesafe at construction:

final case class NonEmptyList[+A](head: A, tail: List[A]) extends NonEmptyCollection[A, List, NonEmptyList] {

I could ask this question on Gitter etc if that is a more appropriate place to ask this. Thanks!

[larsrh]

Closing and re-opening to trigger build

[codecov-io]

Codecov Report

Merging #3362 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #3362   +/-   ##
=======================================
  Coverage   93.34%   93.34%           
=======================================
  Files         378      378           
  Lines        7692     7693    +1     
  Branches      206      204    -2     
=======================================
+ Hits         7180     7181    +1     
  Misses        512      512           

Flag	Coverage Δ
#scala_version_212	93.40% <100.00%> (+<0.01%)	⬆️
#scala_version_213	93.12% <100.00%> (+<0.01%)	⬆️

Impacted Files	Coverage Δ
core/src/main/scala/cats/data/NonEmptyVector.scala	99.25% <100.00%> (+<0.01%)	⬆️
.../src/main/scala/cats/syntax/applicativeError.scala	76.47% <0.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b03df99...c8e9281. Read the comment docs.

[travisbrown]

@ssanj There's a lot of unmotivated inconsistency in the non-empty collections implementations, but in this case the difference is intentional. We want conversions between the standard library collection types and their non-empty cats.data partners to be safe first, but we also want them to be efficient.

It's both safe and efficient to check that a List is non-empty and convert it into a (head, tail) pair. This isn't the case for Vector, which is represented as a balanced tree, which means that taking the tail involves an amount of copying that's significant for our purposes. While it would be ideal to use a representation like NonEmptyList's that makes it impossible to construct invalid values at all, either using public or private APIs, in the case of NonEmptyVector that would be prohibitively inefficient, so we use an internally unsafe representation and exclude the unsafe parts from the public API.

[ssanj]

Thanks for the explanation @travisbrown 👍