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

Streaming.thunk doesn't seem to reevaluate thunk #677

Closed
ceedubs opened this issue Nov 19, 2015 · 2 comments
Closed

Streaming.thunk doesn't seem to reevaluate thunk #677

ceedubs opened this issue Nov 19, 2015 · 2 comments
Assignees

Comments

@ceedubs
Copy link
Contributor

ceedubs commented Nov 19, 2015

The ScalaDoc for Streaming.thunk says that the thunk may not be pure. However, it doesn't seem to support this use-case. In the following example, I would expect the output to be List(1, 2, 3, 4), but it is List(1, 1, 1, 1).

scala> var i = 0
i: Int = 0

scala> Streaming.thunk{() => i += 1; i}.take(4).toList
res6: List[Int] = List(1, 1, 1, 1)

Here is a test that helped me catch this:

  test("thunk is evaluated for each item") {
    // don't want the stream to be too big
    implicit val arbInt = Arbitrary(Gen.choose(-10, 20))
    forAll { (start: Int, end: Int) =>
      var i = start - 1
      val stream = Streaming.thunk{ () => i += 1; i}.takeWhile(_ <= end)
      stream.toList should === ((start to end).toList)
    }
  }
ceedubs added a commit to ceedubs/cats that referenced this issue Nov 19, 2015
I haven't included the one that I posted in the description of typelevel#677
since I'm not yet sure what the right fix for it is.
@ceedubs
Copy link
Contributor Author

ceedubs commented Nov 19, 2015

@non since you are most familiar with Streaming, would you want to take a look at this?

@non
Copy link
Contributor

non commented Nov 19, 2015

Sure!

non added a commit that referenced this issue Nov 19, 2015
The .thunk method is not supposed to memoize the results,
allowing the caller to provide an impure function which
returns different values on different calls.

However, .thunk's definition used .knot with memoization,
which completely defeated the point. This commit removes
the memoization allowing this method to function correctly.

Fixes #677.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants