-
-
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 get for Foldable #1464
Adding get for Foldable #1464
Changes from 2 commits
737e5de
d94f5e8
c829136
6dcea43
f093fb1
71d4fd6
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 |
---|---|---|
|
@@ -209,6 +209,11 @@ private[data] sealed abstract class IorInstances0 { | |
def foldRight[B, C](fa: A Ior B, lc: Eval[C])(f: (B, Eval[C]) => Eval[C]): Eval[C] = | ||
fa.foldRight(lc)(f) | ||
|
||
override def size[B](fa: A Ior B): Long = fa.fold(_ => 0L, _ => 1L, (_, _) => 1L) | ||
|
||
override def get[B](fa: A Ior B)(idx: Long): Option[B] = | ||
if (idx == 0L) fa.fold(_ => None, Some(_), (_, b) => Some(b)) else None | ||
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 can use |
||
|
||
override def forall[B](fa: Ior[A, B])(p: (B) => Boolean): Boolean = fa.forall(p) | ||
|
||
override def exists[B](fa: Ior[A, B])(p: (B) => Boolean): Boolean = fa.exists(p) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -117,6 +117,9 @@ private[data] sealed trait OneAndInstances extends OneAndLowPriority2 { | |
new NonEmptyReducible[OneAnd[F, ?], F] { | ||
override def split[A](fa: OneAnd[F, A]): (A, F[A]) = (fa.head, fa.tail) | ||
|
||
override def get[A](fa: OneAnd[F, A])(idx: Long): Option[A] = | ||
if (idx == 0L) Some(fa.head) else F.get(fa.tail)(idx - 1L) | ||
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 can add this implementation to 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. Done. I know there isn't a test currently; but I think that if such a test exists it should be comparing the default implementation to any overrides which exist. Do you mind if we put that on a different issue because I think that the consistency of the methods is important. |
||
|
||
override def size[A](fa: OneAnd[F, A]): Long = 1 + F.size(fa.tail) | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -404,6 +404,9 @@ private[data] sealed abstract class ValidatedInstances2 { | |
override def size[A](fa: Validated[E, A]): Long = | ||
fa.fold(_ => 0L, _ => 1L) | ||
|
||
override def get[A](fa: Validated[E, A])(idx: Long): Option[A] = | ||
if (idx == 0L) fa.fold(_ => None, Some(_)) else None | ||
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 can use |
||
|
||
override def foldMap[A, B](fa: Validated[E, A])(f: A => B)(implicit B: Monoid[B]): B = | ||
fa.fold(_ => B.empty, f) | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -70,6 +70,16 @@ trait ListInstances extends cats.kernel.instances.ListInstances { | |
G.map2Eval(f(a), lglb)(_ :: _) | ||
}.value | ||
|
||
@tailrec | ||
override def get[A](fa: List[A])(idx: Long): Option[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. can I request we avoid fa match {
case Nil => None
case h :: tail =>
if (idx < 0) None
else if (idx == 0) h
else get(tail)(idx - 1)
} 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. done. |
||
fa match { | ||
case Nil => None | ||
case h :: tail => | ||
if (idx < 0) None | ||
else if (idx == 0) Some(h) | ||
else get(tail)(idx - 1) | ||
} | ||
|
||
override def exists[A](fa: List[A])(p: A => Boolean): Boolean = | ||
fa.exists(p) | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -91,6 +91,13 @@ trait TryInstances extends TryInstances1 { | |
override def reduceRightOption[A](fa: Try[A])(f: (A, Eval[A]) => Eval[A]): Eval[Option[A]] = | ||
Now(fa.toOption) | ||
|
||
override def get[A](fa: Try[A])(idx: Long): Option[A] = | ||
fa match { | ||
case Failure(_) => None | ||
case Success(a) => | ||
if (idx == 0L) Some(a) else None | ||
} | ||
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. Nitpicking: maybe like in if (idx == 0L) fa.toOption else None |
||
|
||
override def size[A](fa: Try[A]): Long = | ||
fa match { | ||
case Failure(_) => 0L | ||
|
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.
Has it not been investigated, whether it's possible to do this with
foldRight
?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.
If we are going to add this, I think we need to implement the faster thing for Vector, NonEmpty*, List, etc...
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.
@edmundnoble I don't think it's possible to do it with
foldRight
unless we choose to have a mutable index outside of the loop, since the thefoldRight
only 'pulls'A
rather than the accumulation.I've got no personal objections to doing it with a mutable
var
, but what are your thoughts?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.
For the default implementation I think
.foldLeft
is probably the best.If you built a custom monadic data type with the right semantics I think you could use
.foldM
to implement.get
with short-circuiting. But I'm not sure it's worth doing compared to just letting data types override the method if desired.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.
The implementation now uses
foldM
overEither
, looks good to me. 👍