Skip to content
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

NonEmptyList work in progress [do-not-merge-yet] #1120

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
159 changes: 159 additions & 0 deletions core/src/main/scala/cats/data/NonEmptyList.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,159 @@
package cats
package data

import cats.std.list._
//it needs a Functor
//it needs semigroup - combine, filter

/**
* A data type which represents a single element (head) and some other
* structure (tail).
*/
final case class NonEmptyList[A](head: A, tail: List[A] = List[A]()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer not to have a default parameter here -- we can support calling with one argument via an additional apply method on the companion object.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 agreed.
@ceedubs had also pointed it out here #1120 (comment)
to have apply method in instances

def apply[A](head: A, tail: A*) = NonEmptyList(head, tail.toList)


/**
* Return the head and tail into a single list
*/
def toList: List[A] = head :: tail

/**
* remove elements not matching the predicate
*/
def filter(p: A => Boolean): List[A] = {
val rest = tail.filter(p)
if (p(head)) head :: rest else rest
Copy link
Contributor

@yilinwei yilinwei Jun 13, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be tempted to go with (head :: tail).filter(p) which I think is toList.filter(p)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually prefer being explicit and not creating a list when not necessary. I'll leave it to @WarFox to decide which way to go.

}

/**
* Append another NonEmptyList
*/
def combine(other: NonEmptyList[A]): NonEmptyList[A] =
NonEmptyList(head, MonadCombine[List].combineK(tail, other.head :: other.tail))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this would be simpler as:

NonEmptyList(head, tail ::: other.toList)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

very much cleaner


/**
* Find the first element matching the predicate, if one exists
*/
def find(p: A => Boolean): Option[A] =
if (p(head)) Some(head) else tail.find(p)

Copy link
Contributor

@yilinwei yilinwei Jun 13, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, toList.find(p)

/**
* Check whether at least one element satisfies the predicate
*/
def exists(p: A => Boolean): Boolean =
p(head) || tail.exists(p)

/**
* Check whether all elements satisfy the predicate
*/
def forall(p: A => Boolean): Boolean =
p(head) && tail.exists(p)

/**
* Left-associative fold on the structure using f.
*/
def foldLeft[B](b: B)(f: (B, A) => B): B =
(head :: tail).foldLeft(b)(f)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this might be better-written as tail.foldLeft(f(b, head))(f).

Copy link
Author

@WarFox WarFox Jun 13, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is tail.foldLeft(f(b, head))(f) same as (head :: tail).foldLeft(b)(f)?

I'm quite new to this, it's a bit confusing. Sorry!
Any documentation that you can point to?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

got it!
First iteration of f will be f(b, head), in (head :: tail).foldLeft(b)(f)


/**
* Right-associative fold on the structure using f.
*/
def foldRight[B](lb: Eval[B])(f: (A, Eval[B]) => Eval[B]): Eval[B] =
(head :: tail).foldRight(lb)(f)

/**
* Applies f to all the elements of the structure
*/
def map[B](f: A => B): NonEmptyList[B] =
NonEmptyList(f(head), tail.map(f))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to have variance on this particular method def map[AA <: A, B](f: AA => B): NonEmptyList[B] so that you can pass a function for a supertype of A into map.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks, I shall try that

}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think in the companion object (when it's created), it would be nice to have a def apply[A](head: A, tail: A*) = NonEmptyList(head, tail.toList). That way people can do something like NonEmptyList(1, 2, 3, 4) as they can with most other collections.


private[data] sealed trait NonEmptyListInstances extends NonEmptyListLowPriority2 {

implicit def catsDataEqForNonEmptyList[A](implicit A: Eq[A]): Eq[NonEmptyList[A]] =
new Eq[NonEmptyList[A]]{
def eqv(x: NonEmptyList[A], y: NonEmptyList[A]): Boolean = x === y
}

implicit def catsDataShowForNonEmptyList[A](implicit A: Show[A]): Show[NonEmptyList[A]] =
Show.show[NonEmptyList[A]](_.show)

implicit def catsDataSemigroupKForNonEmptyList[A]: SemigroupK[NonEmptyList[?]] =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unnecessary type parameter

implicit val catsDataSemigroupKForNonEmptyList: SemigroupK[NonEmptyList] =

new SemigroupK[NonEmptyList[?]] {
def combineK[A](a: NonEmptyList[A], b: NonEmptyList[A]): NonEmptyList[A] =
a combine b
}

implicit def catsDataSemigroupForNonEmptyList[A]: Semigroup[NonEmptyList[A]] =
catsDataSemigroupKForNonEmptyList[F].algebra

implicit def catsDataReducibleForNonEmptyList[A]: Reducible[NonEmptyList[?]] =
new NonEmptyReducible[NonEmptyList[?]] {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that NonEmptyList only takes one parameter, we should be able to remove the ? here.
Reducible[NonEmptyList] should work fine.

override def split[A](fa: NonEmptyList[A]): (A, F[A]) = (fa.head, fa.tail)
}

implicit def catsDataMonadForNonEmptyList[A]: Monad[NonEmptyList[?]] =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unnecessary type parameter

implicit val catsDataMonadForNonEmptyList: Monad[NonEmptyList] =

new Monad[NonEmptyList[?]] {
override def map[A, B](fa: NonEmptyList[F, A])(f: A => B): NonEmptyList[F, B] =
fa map f

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NonEmptyList should have only one type parameter, so this should be NonEmptyList[B] instead of NonEmptyList[F, B]. This should cause compilation errors.

def pure[A](x: A): NonEmptyList[F, A] =
NonEmptyList(x, monad.empty)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's just use Nil instead of monad.empty here, since we know we are talking about List.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the detailed review :)
this is motivational
I shall do this changes tonight


def flatMap[A, B](fa: NonEmptyList[F, A])(f: A => NonEmptyList[F, B]): NonEmptyList[F, B] = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we move this method (and other type class methods) to NonEmptyList and then reference them in the instances? I think that will make the scaladoc for NonEmptyList a bit nicer.

val end = monad.flatMap(fa.tail) { a =>
val fa = f(a)
monad.combineK(monad.pure(fa.head), fa.tail)
}
val fst = f(fa.head)
NonEmptyList(fst.head, monad.combineK(fst.tail, end))
Copy link
Contributor

@yilinwei yilinwei Jun 13, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we know the concrete type we can write this a little nicer as:

val xs = f(head) ++ tail.flatMap(f.andThen(_.toList))
NonEmptyList(xs.head, xs.tail)

}
}
}

trait NonEmptyListLowPriority0 {
implicit val nelComonad: Comonad[NonEmptyList[List, ?]] =
new Comonad[NonEmptyList[List, ?]] {
def coflatMap[A, B](fa: NonEmptyList[List, A])(f: NonEmptyList[List, A] => B): NonEmptyList[List, B] = {
@tailrec def consume(as: List[A], buf: ListBuffer[B]): List[B] =
as match {
case Nil => buf.toList
case a :: as => consume(as, buf += f(NonEmptyList(a, as)))
}
NonEmptyList(f(fa), consume(fa.tail, ListBuffer.empty))
}

def extract[A](fa: NonEmptyList[List, A]): A =
fa.head

def map[A, B](fa: NonEmptyList[List, A])(f: A => B): NonEmptyList[List, B] =
fa map f
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since a Comonad is a Functor, the Comonad here can extend a Functor, in which case you won't need to rewrite map. The same is true for CoflatMap.

trait NonEmptyListComonad extends Comonad[NonEmptyList] 
   with NonEmptyListFunctor with NonEmptyListCoflatMap {
   //extract is the only method you need to write
   def extract[A](fa: NonEmptyList[A]): A = fa.head
}

It would be good to change the catsDataFunctorForNonEmptyList to a NonEmptyListFunctor trait for this purpose

Copy link
Contributor

@ceedubs ceedubs Jun 14, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For OneAnd, where F is abstract, you have to break down a bunch of separate instances (since F may only have Functor or it may have a Monad, etc). Since we are now specializing to List, I think we can probably get by without creating intermediate traits like NonEmptyListFunctor. I think we can probably have a bunch of instances smashed into one like we do here for List (only in this case we will also have Comonad, Reducible, etc.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ceedubs wdyt of delegating applicable methods to the List instance and calling NonEmptyList(list.head, list.tail)?


trait NonEmptyListLowPriority1 extends NonEmptyListLowPriority0 {
implicit def catsDataFunctorForNonEmptyList[F[_]](implicit F: Functor[F]): Functor[NonEmptyList[F, ?]] =
new Functor[NonEmptyList[F, ?]] {
def map[A, B](fa: NonEmptyList[F, A])(f: A => B): NonEmptyList[F, B] =
fa map f
}

}

trait NonEmptyListLowPriority2 extends NonEmptyListLowPriority1 {
implicit def catsDataTraverseForNonEmptyList[F[_]](implicit F: Traverse[F]): Traverse[NonEmptyList[F, ?]] =
new Traverse[NonEmptyList[F, ?]] {
def traverse[G[_], A, B](fa: NonEmptyList[F, A])(f: (A) => G[B])(implicit G: Applicative[G]): G[NonEmptyList[F, B]] = {
G.map2Eval(f(fa.head), Always(F.traverse(fa.tail)(f)))(NonEmptyList(_, _)).value
}

def foldLeft[A, B](fa: NonEmptyList[F, A], b: B)(f: (B, A) => B): B = {
fa.foldLeft(b)(f)
}

def foldRight[A, B](fa: NonEmptyList[F, A], lb: Eval[B])(f: (A, Eval[B]) => Eval[B]): Eval[B] = {
fa.foldRight(lb)(f)
}
}
}

object NonEmptyList extends NonEmptyListInstances
13 changes: 7 additions & 6 deletions core/src/main/scala/cats/data/package.scala
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
package cats

package object data {
type NonEmptyList[A] = OneAnd[List, A]
// type NonEmptyList[A] = OneAnd[List, A]
type NonEmptyVector[A] = OneAnd[Vector, A]
type NonEmptyStream[A] = OneAnd[Stream, A]
type ValidatedNel[E, A] = Validated[NonEmptyList[E], A]

def NonEmptyList[A](head: A, tail: List[A] = Nil): NonEmptyList[A] =
OneAnd(head, tail)
def NonEmptyList[A](head: A, tail: A*): NonEmptyList[A] =
OneAnd[List, A](head, tail.toList)
// def NonEmptyList[A](head: A, tail: List[A] = Nil): NonEmptyList[A] =
// OneAnd(head, tail)
// def NonEmptyList[A](head: A, tail: A*): NonEmptyList[A] =
// OneAnd[List, A](head, tail.toList)

def NonEmptyVector[A](head: A, tail: Vector[A] = Vector.empty): NonEmptyVector[A] =
OneAnd(head, tail)
Expand All @@ -21,6 +21,7 @@ package object data {
def NonEmptyStream[A](head: A, tail: A*): NonEmptyStream[A] =
OneAnd(head, tail.toStream)

/*
object NonEmptyList {
def fromReducible[F[_], A](fa: F[A])(implicit F: Reducible[F]): Eval[NonEmptyList[A]] =
F.reduceRightTo(fa)(a => NonEmptyList(a, Nil)) { (a, lnel) =>
Expand All @@ -32,7 +33,7 @@ package object data {
case (h :: t) => Some(OneAnd(h, t))
case Nil => None
}
}
}*/

type ReaderT[F[_], A, B] = Kleisli[F, A, B]
val ReaderT = Kleisli
Expand Down
165 changes: 165 additions & 0 deletions tests/src/test/scala/cats/tests/NonEmptyListTests.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,165 @@
package cats
package tests

import cats.kernel.laws.{GroupLaws, OrderLaws}

import cats.data.{NonEmptyList, NonEmptyList}
import cats.laws.discipline.{ComonadTests, FunctorTests, SemigroupKTests, FoldableTests, MonadTests, SerializableTests, CartesianTests, TraverseTests, ReducibleTests}
import cats.laws.discipline.arbitrary._

class NonEmptyListTests extends CatsSuite {
// Lots of collections here.. telling ScalaCheck to calm down a bit
implicit override val generatorDrivenConfig: PropertyCheckConfiguration =
PropertyCheckConfig(maxSize = 5, minSuccessful = 20)

checkAll("NonEmptyList[List, Int]", OrderLaws[NonEmptyList[List, Int]].eqv)

checkAll("NonEmptyList[List, Int] with Option", TraverseTests[NonEmptyList[List, ?]].traverse[Int, Int, Int, Int, Option, Option])
checkAll("Traverse[NonEmptyList[List, A]]", SerializableTests.serializable(Traverse[NonEmptyList[List, ?]]))

checkAll("NonEmptyList[List, Int]", ReducibleTests[NonEmptyList[List, ?]].reducible[Option, Int, Int])
checkAll("Reducible[NonEmptyList[List, ?]]", SerializableTests.serializable(Reducible[NonEmptyList[List, ?]]))

implicit val iso = CartesianTests.Isomorphisms.invariant[NonEmptyList[ListWrapper, ?]](NonEmptyList.catsDataFunctorForNonEmptyList(ListWrapper.functor))

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need 2 iso's if the second one resolves.

// Test instances that have more general constraints
{
implicit val monadCombine = ListWrapper.monadCombine
checkAll("NonEmptyList[ListWrapper, Int]", CartesianTests[NonEmptyList[ListWrapper, ?]].cartesian[Int, Int, Int])
checkAll("Cartesian[NonEmptyList[ListWrapper, A]]", SerializableTests.serializable(Cartesian[NonEmptyList[ListWrapper, ?]]))
}

{
implicit val functor = ListWrapper.functor
checkAll("NonEmptyList[ListWrapper, Int]", FunctorTests[NonEmptyList[ListWrapper, ?]].functor[Int, Int, Int])
checkAll("Functor[NonEmptyList[ListWrapper, A]]", SerializableTests.serializable(Functor[NonEmptyList[ListWrapper, ?]]))
}

{
implicit val monadCombine = ListWrapper.monadCombine
checkAll("NonEmptyList[ListWrapper, Int]", SemigroupKTests[NonEmptyList[ListWrapper, ?]].semigroupK[Int])
checkAll("NonEmptyList[List, Int]", GroupLaws[NonEmptyList[List, Int]].semigroup)
checkAll("SemigroupK[NonEmptyList[ListWrapper, A]]", SerializableTests.serializable(SemigroupK[NonEmptyList[ListWrapper, ?]]))
checkAll("Semigroup[NonEmptyList[Int]]", SerializableTests.serializable(Semigroup[NonEmptyList[List, Int]]))
}

{
implicit val foldable = ListWrapper.foldable
checkAll("NonEmptyList[ListWrapper, Int]", FoldableTests[NonEmptyList[ListWrapper, ?]].foldable[Int, Int])
checkAll("Foldable[NonEmptyList[ListWrapper, A]]", SerializableTests.serializable(Foldable[NonEmptyList[ListWrapper, ?]]))
}

{
// Test functor and subclasses don't have implicit conflicts
implicitly[Functor[NonEmptyList]]
implicitly[Monad[NonEmptyList]]
implicitly[Comonad[NonEmptyList]]
}

implicit val iso2 = CartesianTests.Isomorphisms.invariant[NonEmptyList[List, ?]]

checkAll("NonEmptyList[Int]", MonadTests[NonEmptyList].monad[Int, Int, Int])
checkAll("Monad[NonEmptyList[A]]", SerializableTests.serializable(Monad[NonEmptyList]))

checkAll("NonEmptyList[Int]", ComonadTests[NonEmptyList].comonad[Int, Int, Int])
checkAll("Comonad[NonEmptyList[A]]", SerializableTests.serializable(Comonad[NonEmptyList]))

test("Show is not empty and is formatted as expected") {
forAll { (nel: NonEmptyList[Int]) =>
nel.show.nonEmpty should === (true)
nel.show.startsWith("NonEmptyList(") should === (true)
nel.show should === (implicitly[Show[NonEmptyList[Int]]].show(nel))
nel.show.contains(nel.head.show) should === (true)
}
}

test("Show is formatted correctly") {
val nonEmptyList = NonEmptyList("Test", Nil)
nonEmptyList.show should === ("NonEmptyList(Test, List())")
}

test("Creating NonEmptyList + unwrap is identity") {
forAll { (i: Int, tail: List[Int]) =>
val list = i :: tail
val nonEmptyList = NonEmptyList(i, tail: _*)
list should === (nonEmptyList.unwrap)
}
}

test("NonEmptyList#filter is consistent with List#filter") {
forAll { (nel: NonEmptyList[Int], p: Int => Boolean) =>
val list = nel.unwrap
nel.filter(p) should === (list.filter(p))
}
}

test("NonEmptyList#find is consistent with List#find") {
forAll { (nel: NonEmptyList[Int], p: Int => Boolean) =>
val list = nel.unwrap
nel.find(p) should === (list.find(p))
}
}

test("NonEmptyList#exists is consistent with List#exists") {
forAll { (nel: NonEmptyList[Int], p: Int => Boolean) =>
val list = nel.unwrap
nel.exists(p) should === (list.exists(p))
}
}

test("NonEmptyList#forall is consistent with List#forall") {
forAll { (nel: NonEmptyList[Int], p: Int => Boolean) =>
val list = nel.unwrap
nel.forall(p) should === (list.forall(p))
}
}

test("NonEmptyList#map is consistent with List#map") {
forAll { (nel: NonEmptyList[Int], p: Int => String) =>
val list = nel.unwrap
nel.map(p).unwrap should === (list.map(p))
}
}

test("reduceLeft consistent with foldLeft") {
forAll { (nel: NonEmptyList[Int], f: (Int, Int) => Int) =>
nel.reduceLeft(f) should === (nel.tail.foldLeft(nel.head)(f))
}
}

test("reduceRight consistent with foldRight") {
forAll { (nel: NonEmptyList[Int], f: (Int, Eval[Int]) => Eval[Int]) =>
nel.reduceRight(f).value should === (nel.tail.foldRight(nel.head)((a, b) => f(a, Now(b)).value))
}
}

test("reduce consistent with fold") {
forAll { (nel: NonEmptyList[Int]) =>
nel.reduce should === (nel.fold)
}
}

test("reduce consistent with reduceK") {
forAll { (nel: NonEmptyList[Option[Int]]) =>
nel.reduce(SemigroupK[Option].algebra[Int]) should === (nel.reduceK)
}
}

test("reduceLeftToOption consistent with foldLeft + Option") {
forAll { (nel: NonEmptyList[Int], f: Int => String, g: (String, Int) => String) =>
val expected = nel.tail.foldLeft(Option(f(nel.head))) { (opt, i) =>
opt.map(s => g(s, i))
}
nel.reduceLeftToOption(f)(g) should === (expected)
}
}

test("reduceRightToOption consistent with foldRight + Option") {
forAll { (nel: NonEmptyList[Int], f: Int => String, g: (Int, Eval[String]) => Eval[String]) =>
val expected = nel.tail.foldRight(Option(f(nel.head))) { (i, opt) =>
opt.map(s => g(i, Now(s)).value)
}
nel.reduceRightToOption(f)(g).value should === (expected)
}
}
}