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

shift and rehash methods in Array and Hash #3142

Closed
morallo opened this issue Jul 16, 2015 · 9 comments
Closed

shift and rehash methods in Array and Hash #3142

morallo opened this issue Jul 16, 2015 · 9 comments

Comments

@morallo
Copy link

morallo commented Jul 16, 2015

Currently it throws a "NotImplementedError" in MapJavaProxy.java, and it is not defined in ArrayJavaProxy.java

However, the methods seem to be implemented in RubyHash.java and RubyArray.java

Is it a matter of connecting pipes, or am I missing something? Maybe the current implementation in RubyArray and RubyHash does not work?

@headius
Copy link
Member

headius commented Jul 16, 2015

Java Maps do not support rehash, so that's why we raise NotImplementedError. I suppose we could just do nothing, since rehash is largely an invisible operation.

For Java arrays, shift has no meaning; they are fixed-size. I think in this case we need to preserve the NotImpl error.

@headius
Copy link
Member

headius commented Jul 16, 2015

For Java arrays, shift has no meaning; they are fixed-size. I think in this case we need to preserve the NotImpl error.

I mean we should leave it unimplemented. Java arrays are not like Ruby arrays. If you want a Ruby array, call to_a.

@headius
Copy link
Member

headius commented Jul 16, 2015

May I ask why you are calling these methods against Java maps? Perhaps we can help you find an alternative, because at least in the case of shift I don't think we can do anything.

@headius
Copy link
Member

headius commented Jul 16, 2015

I should point out that rehash does work when the particular Java Map supports it, but it doesn't exist for all Map impls:

[] ~/projects/jruby $ jruby -e "java.util.Hashtable.new.rehash"

[] ~/projects/jruby $ jruby -e "java.util.HashMap.new.rehash"
NotImplementedError: rehash method is not implemented in a Java Map backed object
  rehash at org/jruby/java/proxies/MapJavaProxy.java:310
   <top> at -e:1

@morallo
Copy link
Author

morallo commented Jul 16, 2015

Thx for the reply.

I'm writing a custom plugin for logstash.
The events that I get when my code is called by the main application are Java maps.

If I use to_a, I'm afraid I will get a copy of the event, and won't be able to modify referenced map itself.

@headius
Copy link
Member

headius commented Jul 16, 2015

Re maps: I can make rehash work, but there's no equivalent to shift for Java maps.

Re arrays: Java arrays have fixed size, so size-altering methods will never have an equivalent. Not really anything we can do about that :-\

headius added a commit that referenced this issue Jul 16, 2015
* Make the default rehash impl no-op. If the map supports rehash,
  the correct Java method will be bound instead.
* Make the default shift have a better error explaining that Java
  Map does not generally preserve insertion order.
@headius
Copy link
Member

headius commented Jul 16, 2015

I've pushed the rehash change, and improved the shift error message. That's about all we can do, so I'm marking this fixed.

@headius headius closed this as completed Jul 16, 2015
headius added a commit that referenced this issue Jul 16, 2015
* Make the default rehash impl no-op. If the map supports rehash,
  the correct Java method will be bound instead.
* Make the default shift have a better error explaining that Java
  Map does not generally preserve insertion order.
@morallo
Copy link
Author

morallo commented Jul 16, 2015

Ok, thx anyway!

@headius
Copy link
Member

headius commented Jul 16, 2015

Oh, one last change I will do...if I mark the default rehash and shift as unimplemented, they should not show up in respond_to? checks.

headius added a commit that referenced this issue Jul 16, 2015
Allows using respond_to? to check if they do anything. #3142
headius added a commit that referenced this issue Jul 16, 2015
Allows using respond_to? to check if they do anything. #3142
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

2 participants