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

Implemented Enumerator#feed method #2486

Merged
merged 3 commits into from Jan 22, 2015

Conversation

Projects
None yet
3 participants
@Who828
Copy link
Contributor

Who828 commented Jan 20, 2015

This PR implements Enumerator#feed and addresses #1571. However the behaviour is not same as MRI because of the difference in implementation of next in JRuby and MRI.

Here is an example to demonstrate the difference,

MRI

a = [1, 2, 3]
m = a.to_enum(:map!)

m.next # a -> [1, 2, 3], returns 1
m.next # a -> [nil, 2, 3], returns 2
m.next # a -> [nil, nil, 3], returns 3
m.next # a -> [nil, nil, nil],  throws StopIteration exception and calls the yield/method

JRuby

a = [1, 2, 3]
m = a.to_enum(:map!)

m.next # a -> [nil, 2, 3], returns 1
m.next # a -> [nil, nil, 3], returns 2
m.next # a -> [nil, nil, nil], returns 3
m.next # a -> [nil, nil, nil],  throws StopIteration exception and doesn't call yield/method

So basically when you call next first time in MRI, it actually initialises the block but doesn't call it yet unlike JRuby.

So because of this I haven't included test_feed specs in TestEnumerable (test:mri), as they fail because of the different order of evaluation of yield/method.

Though this PR does implement feed correctly for JRuby's next implementation, so I am raising it.

@headius

This comment has been minimized.

Copy link
Member

headius commented Jan 21, 2015

A few improvements you should do:

  • If you must call getRuntime, cache the result. Don't keep calling it everywhere you need a runtime.
  • Better: get it from ThreadContext.runtime (and cache it in a local for that too).
  • nil is accessible more cheaply as context.nil
@enebo

This comment has been minimized.

Copy link
Member

enebo commented Jan 21, 2015

And I said this in other PR about no curlies on single line ifs. So since there are other things to change do that as well.

@Who828

This comment has been minimized.

Copy link
Contributor Author

Who828 commented Jan 21, 2015

Made the changes based on the feedback.

@enebo enebo added this to the 9.0.0.0.pre2 milestone Jan 22, 2015

enebo added a commit that referenced this pull request Jan 22, 2015

Merge pull request #2486 from Who828/impl_feed
Implemented Enumerator#feed method

@enebo enebo merged commit ad95b0b into jruby:master Jan 22, 2015

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.