Implement Map interface in ScriptableObject

RESOLVED FIXED

Status

Rhino
Core
RESOLVED FIXED
9 years ago
7 years ago

People

(Reporter: Bill Wallace, Unassigned)

Tracking

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

9 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-GB; rv:1.9) Gecko/2008052906 Firefox/3.0
Build Identifier: 

It would be quite useful if ScriptableObject implemented at least a minimal version of the Map interface - containsKey, get, put, remove at a minimum, but also perhaps a read-only version of the various set views of the object.  This would allow accessing a NativeObject much more cleanly than having to know about the Scriptable Interface etc, and it could be accessed by the many classes that already can deal with java Maps.  In terms of general behaviour, this is much more useful than having ScriptableObject be implemented in terms of a HashMap, and should not cause any performance degredation.

Reproducible: Always

Steps to Reproduce:
1.
2.
3.

Comment 1

9 years ago
Created attachment 347951 [details] [diff] [review]
patch and test case

Patch and testcase for implementing java.util.Map in ScriptableObject. Currently, put() is not supported (mainly because of object wrapping concerns, which probably isn't hard to implement), while remove() is, both directly and through the key/value/entry collections.

Comment 2

9 years ago
Created attachment 347953 [details] [diff] [review]
improved patch and testcase

Simplified patch, improved test case.
Attachment #347951 - Attachment is obsolete: true

Comment 3

9 years ago
Committed to HEAD:

Checking in src/org/mozilla/javascript/ScriptableObject.java;
/cvsroot/mozilla/js/rhino/src/org/mozilla/javascript/ScriptableObject.java,v  <--  ScriptableObject.java
new revision: 1.140; previous revision: 1.139
done
RCS file: /cvsroot/mozilla/js/rhino/testsrc/org/mozilla/javascript/tests/Bug448816Test.java,v
done
Checking in testsrc/org/mozilla/javascript/tests/Bug448816Test.java;
/cvsroot/mozilla/js/rhino/testsrc/org/mozilla/javascript/tests/Bug448816Test.java,v  <--  Bug448816Test.java
initial revision: 1.1
done
Status: UNCONFIRMED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED

Comment 4

9 years ago
Created attachment 349447 [details] [diff] [review]
additional patch

additional patch to make the code compile and work with Java 1.5.

Comment 5

9 years ago
Some thoughts and observations: 

First, maybe the implementation of Map should happen in NativeObject, not ScriptableObject. ScriptableObject is the parent class of virtually every scriptable object in Rhino, so implementing Map there may be a bit too 'broad', and I think the idea is to have this feature explicitly for plain JS Objects.

Second, I don't think it would be difficult to implement write access over the Map interface. I think all we'd have to make sure is that values are properly wrapped using the context's WrapFactory. 

I was trying to reopen this bug until these issues are sorted out but I can't - don't know if this is due to a bugzilla bug or some missing permissions.

Comment 6

7 years ago
java.util.Map implementation has moved from ScriptableObject to NativeObject in the process of fixing bug 466207 (implementing java.util.List interface in NativeArray). The primary reason was that it is impossible to implement both Map ans List in the same class as they have remove(Object) methods with incompatible return types. Also, implementing Map makes most sense in NativeObject. It's not clear any JS object should be treated as Map when passed to Java code.
You need to log in before you can comment on or make changes to this bug.