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

Comments

Projects
None yet
2 participants
@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

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

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

Improvements to MapJavaProxy for rehash and shift. #3142
* 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

This comment has been minimized.

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 Jul 16, 2015

headius added a commit that referenced this issue Jul 16, 2015

Improvements to MapJavaProxy for rehash and shift. #3142
* 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

This comment has been minimized.

Copy link
Author

morallo commented Jul 16, 2015

Ok, thx anyway!

@headius

This comment has been minimized.

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

Mark default java.util.Map rehash and shift as not implemented.
Allows using respond_to? to check if they do anything. #3142

headius added a commit that referenced this issue Jul 16, 2015

Mark default java.util.Map rehash and shift as not implemented.
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
You can’t perform that action at this time.