-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adding NonEmptyVector #1137
Adding NonEmptyVector #1137
Changes from 1 commit
c6c8ee0
fb7988f
b715ee7
74e8a40
33b845f
d237bdb
a79bc57
a9ceb60
e35ada2
696e269
e173928
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,69 +3,99 @@ package data | |
|
||
import scala.annotation.tailrec | ||
import scala.collection.immutable.VectorBuilder | ||
import scala.util.Try | ||
import cats.std.vector._ | ||
|
||
/** | ||
* A data type which represents a single element (head) and a Vector | ||
* (tail). This can be used to represent a Vector which is guaranteed | ||
* to not be empty. | ||
* A data type which represents a non empty Vector. | ||
*/ | ||
final case class NonEmptyVector[A] private (vector: Vector[A]) { | ||
final case class NonEmptyVector[A] private (toVector: Vector[A]) { | ||
|
||
def unwrap: Vector[A] = vector | ||
/** Gets the element at the index, if it exists */ | ||
def get(i: Int): Option[A] = | ||
Try(getUnsafe(i)).toOption | ||
|
||
def head: A = vector.head | ||
/** Gets the element at the index, or throws an exception if none exists */ | ||
def getUnsafe(i: Int): A = toVector(i) | ||
|
||
def tail: Vector[A] = vector.tail | ||
/** Updates the element at the index, if it exists */ | ||
def updated(i: Int, a: A): Option[NonEmptyVector[A]] = | ||
Try(updatedUnsafe(i, a)).toOption | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. More performance speculation: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oops I guess that would need to wrap it in a |
||
|
||
/** Updates the element at the index, or throws an exeption if none exists */ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Small typo: exeption -> exception. |
||
def updatedUnsafe(i: Int, a: A): | ||
NonEmptyVector[A] = NonEmptyVector(toVector.updated(i, a)) | ||
|
||
def head: A = toVector.head | ||
|
||
def tail: Vector[A] = toVector.tail | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I feel like it's worth supporting There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Or both? :-) But agree on wanting indexed access. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @non you mean I'd be in favor of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes! Sorry, in my defense it was late! ;) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why not have: def get(i: Int): Option[A]
// This is unsafe, it may throw on out-of-bounds, see `get` for a safe variant.
def apply(i: Int): A for updated I would like: def updatedOption(i: Int, a: A): Option[NonEmptyVector[A]]
// this will throw is i is out of bounds.
def updated(i: Int, a: A): NonEmptyVector[A] I feel like adding There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. alternatively, since There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is tough. I think that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can see your point on safety, but there is also a cost (especially to reviewers of future code) of using names that are unsafe on Vector to be safe here (if they change the type signature). My bias is using Maybe I would say: when we can safely follow standard naming conventions, we should. Otherwise, if we want to add methods that can throw, use
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @johnynek everything that you said sounds good to me. So it sounds like it'll be |
||
/** | ||
* remove elements not matching the predicate | ||
*/ | ||
def filter(f: A => Boolean): Vector[A] = vector.filter(f) | ||
def filter(f: A => Boolean): Vector[A] = toVector.filter(f) | ||
|
||
/** | ||
* Append another NonEmptyVector to this | ||
*/ | ||
def combine(other: NonEmptyVector[A]): NonEmptyVector[A] = NonEmptyVector(vector ++ other.vector) | ||
def concat(other: NonEmptyVector[A]): NonEmptyVector[A] = NonEmptyVector(toVector ++ other.toVector) | ||
|
||
/** | ||
* Append another Vector to this | ||
*/ | ||
def concat(other: Vector[A]): NonEmptyVector[A] = NonEmptyVector(toVector ++ other) | ||
|
||
/** | ||
* find the first element matching the predicate, if one exists | ||
*/ | ||
def find(f: A => Boolean): Option[A] = vector.find(f) | ||
def find(f: A => Boolean): Option[A] = toVector.find(f) | ||
|
||
/** | ||
* Check whether at least one element satisfies the predicate. | ||
*/ | ||
def exists(f: A => Boolean): Boolean = vector.exists(f) | ||
def exists(f: A => Boolean): Boolean = toVector.exists(f) | ||
|
||
/** | ||
* Check whether all elements satisfy the predicate. | ||
*/ | ||
def forall(f: A => Boolean): Boolean = vector.forall(f) | ||
def forall(f: A => Boolean): Boolean = toVector.forall(f) | ||
|
||
/** | ||
* Left-associative fold using f. | ||
*/ | ||
def foldLeft[B](b: B)(f: (B, A) => B): B = | ||
vector.foldLeft(b)(f) | ||
toVector.foldLeft(b)(f) | ||
|
||
/** | ||
* Right-associative fold using f. | ||
*/ | ||
def foldRight[B](lb: Eval[B])(f: (A, Eval[B]) => Eval[B]): Eval[B] = | ||
Eval.defer(Foldable[Vector].foldRight(vector, lb)(f)) | ||
Foldable[Vector].foldRight(toVector, lb)(f) | ||
|
||
/** | ||
* Applies f to all the elements | ||
*/ | ||
def map[B](f: A => B): NonEmptyVector[B] = | ||
NonEmptyVector(vector.map(f)) | ||
NonEmptyVector(toVector.map(f)) | ||
|
||
/** | ||
* Applies f to all elements and combines the result | ||
*/ | ||
def flatMap[B](f: A => NonEmptyVector[B]): NonEmptyVector[B] = | ||
NonEmptyVector(vector.flatMap(a => f(a).vector)) | ||
NonEmptyVector(toVector.flatMap(a => f(a).toVector)) | ||
|
||
/** | ||
* Left-associative reduce using f. | ||
*/ | ||
def reduceLeft(f: (A, A) => A): A = | ||
tail.foldLeft(head)(f) | ||
|
||
/** | ||
* Left-associative reduce using the Semigroup of A | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should remove this comment: "Left-associative" since |
||
*/ | ||
def reduce(implicit S: Semigroup[A]): A = | ||
reduceLeft(S.combine) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. semigroup has |
||
|
||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what about There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think they should be called There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we could add |
||
/** | ||
* Typesafe equality operator. | ||
|
@@ -75,7 +105,7 @@ final case class NonEmptyVector[A] private (vector: Vector[A]) { | |
* equality provided by Eq[_] instances, rather than using the | ||
* universal equality provided by .equals. | ||
*/ | ||
def ===(that: NonEmptyVector[A])(implicit A: Eq[A]): Boolean = Eq[Vector[A]].eqv(vector, that.vector) | ||
def ===(that: NonEmptyVector[A])(implicit A: Eq[A]): Boolean = Eq[Vector[A]].eqv(toVector, that.toVector) | ||
|
||
/** | ||
* Typesafe stringification method. | ||
|
@@ -85,37 +115,29 @@ final case class NonEmptyVector[A] private (vector: Vector[A]) { | |
* universal .toString method. | ||
*/ | ||
def show(implicit A: Show[A]): String = | ||
s"NonEmptyVector(${Show[Vector[A]].show(vector)})" | ||
s"NonEmptyVector(${Show[Vector[A]].show(toVector)})" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this will give us something like |
||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this show correct? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it's better than the |
||
|
||
private[data] sealed trait NonEmptyVectorInstances extends NonEmptyVectorLowPriority2 { | ||
private[data] sealed trait NonEmptyVectorInstances { | ||
|
||
implicit def catsDataEqForNonEmptyVector[A](implicit A: Eq[A]): Eq[NonEmptyVector[A]] = | ||
new Eq[NonEmptyVector[A]]{ | ||
def eqv(x: NonEmptyVector[A], y: NonEmptyVector[A]): Boolean = x === y | ||
} | ||
|
||
implicit def catsDataShowForNonEmptyVector[A](implicit A: Show[A]): Show[NonEmptyVector[A]] = | ||
Show.show[NonEmptyVector[A]](_.show) | ||
implicit val catsDataInstancesForNonEmptyVector: SemigroupK[NonEmptyVector] with Reducible[NonEmptyVector] | ||
with Monad[NonEmptyVector] with Comonad[NonEmptyVector] with Traverse[NonEmptyVector] = | ||
new NonEmptyReducible[NonEmptyVector, Vector] with SemigroupK[NonEmptyVector] with Monad[NonEmptyVector] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this can be a and also I am pretty sure There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we can do 👍 for the |
||
with Comonad[NonEmptyVector] with Traverse[NonEmptyVector] { | ||
|
||
implicit def catsDataSemigroupKForNonEmptyVector: SemigroupK[NonEmptyVector] = | ||
new SemigroupK[NonEmptyVector] { | ||
def combineK[A](a: NonEmptyVector[A], b: NonEmptyVector[A]): NonEmptyVector[A] = | ||
a combine b | ||
} | ||
|
||
implicit def catsDataSemigroupForNonEmptyVector[A]: Semigroup[NonEmptyVector[A]] = | ||
catsDataSemigroupKForNonEmptyVector.algebra | ||
a concat b | ||
|
||
implicit def catsDataReducibleForNonEmptyVector: Reducible[NonEmptyVector] = | ||
new NonEmptyReducible[NonEmptyVector, Vector] { | ||
override def split[A](fa: NonEmptyVector[A]): (A, Vector[A]) = (fa.head, fa.tail) | ||
|
||
override def size[A](fa: NonEmptyVector[A]): Long = 1 + fa.tail.size.toLong | ||
} | ||
|
||
implicit def catsDataMonadForNonEmptyVector: Monad[NonEmptyVector] = | ||
new Monad[NonEmptyVector] { | ||
override def reduceLeft[A](fa: NonEmptyVector[A])(f: (A, A) => A): A = | ||
fa.reduceLeft(f) | ||
|
||
override def reduce[A](fa: NonEmptyVector[A])(implicit A: Semigroup[A]): A = | ||
fa.reduce | ||
|
||
override def map[A, B](fa: NonEmptyVector[A])(f: A => B): NonEmptyVector[B] = | ||
fa map f | ||
|
||
|
@@ -124,14 +146,8 @@ private[data] sealed trait NonEmptyVectorInstances extends NonEmptyVectorLowPrio | |
|
||
def flatMap[A, B](fa: NonEmptyVector[A])(f: A => NonEmptyVector[B]): NonEmptyVector[B] = | ||
fa flatMap f | ||
} | ||
} | ||
|
||
trait NonEmptyVectorLowPriority0 { | ||
|
||
implicit def catsDataComonadForNonEmptyVector: Comonad[NonEmptyVector] = new Comonad[NonEmptyVector] { | ||
|
||
def coflatMap[A, B](fa: NonEmptyVector[A])(f: NonEmptyVector[A] => B): NonEmptyVector[B] = { | ||
def coflatMap[A, B](fa: NonEmptyVector[A])(f: NonEmptyVector[A] => B): NonEmptyVector[B] = { | ||
@tailrec def consume(as: Vector[A], buf: VectorBuilder[B]): Vector[B] = | ||
as match { | ||
case a +: as => consume(as, buf += f(NonEmptyVector(a, as))) | ||
|
@@ -140,40 +156,47 @@ trait NonEmptyVectorLowPriority0 { | |
NonEmptyVector(f(fa), consume(fa.tail, new VectorBuilder[B])) | ||
} | ||
|
||
def extract[A](fa: NonEmptyVector[A]): A = fa.head | ||
|
||
def extract[A](fa: NonEmptyVector[A]): A = fa.head | ||
|
||
def map[A, B](fa: NonEmptyVector[A])(f: A => B): NonEmptyVector[B] = fa map f | ||
} | ||
} | ||
|
||
trait NonEmptyVectorLowPriority1 extends NonEmptyVectorLowPriority0 { | ||
implicit def catsDataFunctorForNonEmptyVector: Functor[NonEmptyVector] = | ||
new Functor[NonEmptyVector] { | ||
def map[A, B](fa: NonEmptyVector[A])(f: A => B): NonEmptyVector[B] = | ||
fa map f | ||
} | ||
|
||
} | ||
|
||
trait NonEmptyVectorLowPriority2 extends NonEmptyVectorLowPriority1 { | ||
implicit def catsDataTraverseForNonEmptyVector: Traverse[NonEmptyVector] = | ||
new Traverse[NonEmptyVector] { | ||
def traverse[G[_], A, B](fa: NonEmptyVector[A])(f: (A) => G[B])(implicit G: Applicative[G]): G[NonEmptyVector[B]] = | ||
G.map2Eval(f(fa.head), Always(Traverse[Vector].traverse(fa.tail)(f)))(NonEmptyVector(_, _)).value | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Slightly nitpicking but the 2 spaces for indentation seem to be missing. |
||
|
||
|
||
def foldLeft[A, B](fa: NonEmptyVector[A], b: B)(f: (B, A) => B): B = | ||
override def foldLeft[A, B](fa: NonEmptyVector[A], b: B)(f: (B, A) => B): B = | ||
fa.foldLeft(b)(f) | ||
|
||
def foldRight[A, B](fa: NonEmptyVector[A], lb: Eval[B])(f: (A, Eval[B]) => Eval[B]): Eval[B] = | ||
override def foldRight[A, B](fa: NonEmptyVector[A], lb: Eval[B])(f: (A, Eval[B]) => Eval[B]): Eval[B] = | ||
fa.foldRight(lb)(f) | ||
} | ||
|
||
implicit def catsDataEqForNonEmptyVector[A](implicit A: Eq[A]): Eq[NonEmptyVector[A]] = | ||
new Eq[NonEmptyVector[A]]{ | ||
def eqv(x: NonEmptyVector[A], y: NonEmptyVector[A]): Boolean = x === y | ||
} | ||
|
||
implicit def catsDataShowForNonEmptyVector[A](implicit A: Show[A]): Show[NonEmptyVector[A]] = | ||
Show.show[NonEmptyVector[A]](_.show) | ||
|
||
implicit def catsDataSemigroupForNonEmptyVector[A]: Semigroup[NonEmptyVector[A]] = | ||
catsDataInstancesForNonEmptyVector.algebra | ||
|
||
} | ||
|
||
object NonEmptyVector extends NonEmptyVectorInstances { | ||
|
||
def apply[A](head: A, tail: Vector[A]): NonEmptyVector[A] = NonEmptyVector(head +: tail) | ||
def apply[A](head: A, tail: Vector[A]): NonEmptyVector[A] = | ||
NonEmptyVector(head +: tail) | ||
|
||
def apply[A](head: A, tail: A*): NonEmptyVector[A] = NonEmptyVector(head +: tail.toVector) | ||
def apply[A](head: A, tail: A*): NonEmptyVector[A] = | ||
NonEmptyVector(head +: tail.toVector) | ||
|
||
def fromVector[A](vector: Vector[A]): Option[NonEmptyVector[A]] = | ||
vector.headOption.map(apply(_, vector.tail)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. More performance speculation: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. +1. I think we should do the horrible, but fast stuff internally when we can prove it is correct (and then test it to be sure). |
||
|
||
def fromVectorUnsafe[A](vector: Vector[A]): NonEmptyVector[A] = | ||
fromVector(vector) match { | ||
case Some(v) => v | ||
case None => | ||
throw new IllegalArgumentException("Cannot create NonEmptyVector from empty vector") | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd probably just do an There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. +1 |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,7 +20,7 @@ class NonEmptyVectorTests extends CatsSuite { | |
checkAll("NonEmptyVector[Int]", ReducibleTests[NonEmptyVector].reducible[Option, Int, Int]) | ||
checkAll("Reducible[NonEmptyVector]", SerializableTests.serializable(Reducible[NonEmptyVector])) | ||
|
||
implicit val iso = CartesianTests.Isomorphisms.invariant[NonEmptyVector](NonEmptyVector.catsDataFunctorForNonEmptyVector) | ||
implicit val iso = CartesianTests.Isomorphisms.invariant[NonEmptyVector] | ||
|
||
// Test instances that have more general constraints | ||
{ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we need to add braces for separate scopes for the following tests. That's just done for tests where there's an abstract |
||
|
@@ -84,46 +84,46 @@ class NonEmptyVectorTests extends CatsSuite { | |
nonEmptyVector.show should === ("NonEmptyVector(Vector(Test))") | ||
} | ||
|
||
test("Creating NonEmptyVector + unwrap is identity") { | ||
test("Creating NonEmptyVector + toVector is identity") { | ||
forAll { (i: Int, tail: Vector[Int]) => | ||
val vector = i +: tail | ||
val nonEmptyVector = NonEmptyVector(i, tail) | ||
vector should === (nonEmptyVector.unwrap) | ||
vector should === (nonEmptyVector.toVector) | ||
} | ||
} | ||
|
||
test("NonEmptyVector#filter is consistent with NonEmptyVector#filter") { | ||
forAll { (nonEmptyVector: NonEmptyVector[Int], p: Int => Boolean) => | ||
val vector = nonEmptyVector.unwrap | ||
val vector = nonEmptyVector.toVector | ||
nonEmptyVector.filter(p) should === (vector.filter(p)) | ||
} | ||
} | ||
|
||
test("NonEmptyVector#find is consistent with NonEmptyVector#find") { | ||
forAll { (nonEmptyVector: NonEmptyVector[Int], p: Int => Boolean) => | ||
val vector = nonEmptyVector.unwrap | ||
val vector = nonEmptyVector.toVector | ||
nonEmptyVector.find(p) should === (vector.find(p)) | ||
} | ||
} | ||
|
||
test("NonEmptyVector#exists is consistent with NonEmptyVector#exists") { | ||
forAll { (nonEmptyVector: NonEmptyVector[Int], p: Int => Boolean) => | ||
val vector = nonEmptyVector.unwrap | ||
val vector = nonEmptyVector.toVector | ||
nonEmptyVector.exists(p) should === (vector.exists(p)) | ||
} | ||
} | ||
|
||
test("NonEmptyVector#forall is consistent with NonEmptyVector#forall") { | ||
forAll { (nonEmptyVector: NonEmptyVector[Int], p: Int => Boolean) => | ||
val vector = nonEmptyVector.unwrap | ||
val vector = nonEmptyVector.toVector | ||
nonEmptyVector.forall(p) should === (vector.forall(p)) | ||
} | ||
} | ||
|
||
test("NonEmptyVector#map is consistent with NonEmptyVector#map") { | ||
forAll { (nonEmptyVector: NonEmptyVector[Int], p: Int => String) => | ||
val vector = nonEmptyVector.unwrap | ||
nonEmptyVector.map(p).unwrap should === (vector.map(p)) | ||
val vector = nonEmptyVector.toVector | ||
nonEmptyVector.map(p).toVector should === (vector.map(p)) | ||
} | ||
} | ||
|
||
|
@@ -168,4 +168,14 @@ class NonEmptyVectorTests extends CatsSuite { | |
nonEmptyVector.reduceRightToOption(f)(g).value should === (expected) | ||
} | ||
} | ||
|
||
test("fromVector returns None when the input vector is empty") { | ||
NonEmptyVector.fromVector(Vector.empty[Int]) should === (Option.empty[NonEmptyVector[Int]]) | ||
} | ||
|
||
test("fromVectorUnsafe throws an exception when the input vector is empty") { | ||
val _ = intercept[IllegalArgumentException] { | ||
NonEmptyVector.fromVectorUnsafe(Vector.empty[Int]) | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that
toVector.lift(i)
would probably be a more efficient implementation (though this is performance speculation without a benchmark).