Renamed lazy foldRight to foldLazy, and made it stack-safe. #150
Conversation
This method introduces a new data type, Fold[A]. This type allows to programmer to decide whether to continue evaluating the fold (and provide an appropriate continuation function), or to return immediately.
|
There are a few things I'd like feedback on with this PR:
|
| def loop(it: Iterator[A], fs: List[B => B]): B = | ||
| if (it.hasNext) { | ||
| val a: A = it.next | ||
| val fb: Fold[B] = f(it.next) |
There was a problem hiding this comment.
Oh wow, great catch! What a horrible bug :/
There was a problem hiding this comment.
I was fooling around with variance annotations and pulled everything out to give manual type annotations -- obviously I failed to clean things up sufficiently.
|
As @xuwei-k's bug report shows, I need to add some tests to exercise this code. |
| * | ||
| * This method evaluates `b` lazily (in some cases it will not be | ||
| * needed), and returns a lazy value. For more information about how | ||
| * this method works see the documentation for Fold[_]. |
There was a problem hiding this comment.
I think you can add a "@see" for the scaladoc
|
@non Good work; I like this. Scalaz has had lots of issues with things like exists and forall not short-circuiting. In fact it looks like the rest (hopefully) were finally cleaned up just 2 days ago: scalaz/scalaz#896 (thanks, @xuwei-k!). I think using this approach as the default on derived I'll hold off on a thumbsup until you've had a chance to write the tests you mentioned, but I wanted to say thanks for putting this together and let you know that I think it looks like a good approach. |
These tests exercise Foldable -- in particular they ensure that the List and Stream instances' foldLazy method is lazy and also stack-safe.
|
@ceedubs Thanks for the feedback! I have some basic tests in, so I feel pretty good about this now. |
|
👍 |
| * | ||
| * This code searches an infinite stream for 77: | ||
| * | ||
| * val found: Lazy[Boolean] = |
There was a problem hiding this comment.
A bit pedantic, but if we wrap the code here in {{{ ... }}} then ScalaDoc will treat it as preformatted text.
There was a problem hiding this comment.
Oh, that's a good point.
These changes should address the comments raised by @tixxit and @julien-truffaut.
|
👍 |
This means that there are only two abstract methods in Foldable[F] now: foldLeft and foldLazy.
There are also a few small changes to make the code a bit more readable, but nothing major. At this point I think the documentation for Foldable is about as good as I can make it.
|
👍 Thanks for all the hard work on this @non, I think this is fantastic |
Renamed lazy foldRight to foldLazy, and made it stack-safe.
I should have merged master into #150, which would have caught this problem earlier. Mea culpa.
This method introduces a new data type, Fold[A]. This type
allows to programmer to decide whether to continue evaluating
the fold (and provide an appropriate continuation function),
or to return immediately.
This fixes #88