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

Java List and array access improvements #561

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

makusuko
Copy link
Contributor

Add a new wrapper calss NativeJavaList that provides natural JS array access for Java lists. Improve NativeJavaArray for better compatability with JS via the addition of indexOf and includes functions. Add a couple of tests for the list and array access.

both NativeJavaArray and NativeJavaList for better compatability with
JS. Add a couple of tests for the list and array access.
@gbrail
Copy link
Collaborator

gbrail commented May 29, 2019

Thanks. I'll take a look at this next week after we finish the 1.7.11 release. One difference between this and other built-in classes is in the way that it adds new methods -- other native classes inherit from IdScriptableObject whereas this one uses a different method. One thing that we might want to do is to make this class work that way too.

Also, a number of the prototype classes on regular arrays should work on Java arrays too -- we should look and see why not.

@makusuko
Copy link
Contributor Author

Yeah, I tried to use a couple of ways to implement the JavaScript functions. The thing is, that the wrapped object should expose all the methods and properties from the object implementing the List interface. For example, in our projects we tend to have business objects, which implement List interface, but have quite a lot of other business logic in them too.

if (arg instanceof Wrapper)
arg = ((Wrapper)arg).unwrap();

int start = 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a test when start is specified as an argument?

var a = java.util.Arrays.asList(1, 2, 3).toArray();
a.indexOf(1, 5);  // -1?
a.indexOf(1, -5); // throw error?


int length = Array.getLength(array);
for (int index = start; index < length; index++)
if (Objects.deepEquals(arg, Array.get(array, index)))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a test to make sure deepEquals is called?

@p-bakker p-bakker added the Java Interop Issues related to the interaction between Java and JavaScript label Jun 30, 2021
Copy link
Contributor

@rPraml rPraml left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a very old PR and goes into the same direction as #830
Personally I can take up the topic again after #1031 is merged

import java.util.ArrayList;
import java.util.List;

public class AccessingJavaList extends TestCase {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test is passing in the current codebase.

public Object get(String id, Scriptable start) {
if (id.equals("length"))
return Integer.valueOf(list.size());
else if (id.equals("includes"))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is only one extension of the list.
I think we were both trying to reach the same goal: See #830
A JavaList should be treated like an Array and provide functions like join, includes etc...

rPraml added a commit to FOCONIS/rhino that referenced this pull request Nov 9, 2021
rPraml added a commit to FOCONIS/rhino that referenced this pull request Nov 9, 2021
@rPraml
Copy link
Contributor

rPraml commented Nov 9, 2021

Most of the test cases SHOULD¹ pass in the current codebase of #1083. (¹ plugins.gradle.com is currently down...)
I've adjusted only one code faeada0 which is correct in my mind. (There is no includes in java.util.List, so you have to use the Array.prototype.includes)
@makusuko If you agree, this PR can be closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Java Interop Issues related to the interaction between Java and JavaScript
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants