Skip to content

Implemented Enumerator#feed method#2486

Merged
enebo merged 3 commits intojruby:masterfrom
Who828:impl_feed
Jan 22, 2015
Merged

Implemented Enumerator#feed method#2486
enebo merged 3 commits intojruby:masterfrom
Who828:impl_feed

Conversation

@Who828
Copy link
Contributor

@Who828 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
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
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
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
Implemented Enumerator#feed method
@enebo enebo merged commit ad95b0b into jruby:master Jan 22, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants