Add methods from List/Map to Listing/Mapping
#683
Conversation
| var keys = getAllKeys(); | ||
| var builder = new VmObjectBuilder(keys.getLength()); | ||
| for (var key : keys) { | ||
| var member = VmUtils.readMember(this, key); |
There was a problem hiding this comment.
| var member = VmUtils.readMember(this, key); | |
| var value = VmUtils.readMember(this, key); |
| public VmDynamic toDynamic() { | ||
| var keys = getAllKeys(); | ||
| var builder = new VmObjectBuilder(keys.getLength()); | ||
| for (var key : keys) { |
There was a problem hiding this comment.
How about using self.forceAndIterateMemberValues() here? This should be much more efficient than calling readMember() repeatedly because it iterates the amend hierarchy only once.
Zooming out, how about returning a Dynamic whose members delegate to the mapping? This would avoid the need to force the mapping, which seems desirable.
Same comment applies to other similar methods.
|
|
||
| /// Converts this mapping to a [Map]. | ||
| external function toMap(): Map<Key, Value> | ||
|
|
||
| /// Converts this mapping to a [Dynamic] object. | ||
| external function toDynamic(): Dynamic |
There was a problem hiding this comment.
How about documenting that late binding and Mapping.default are not preserved?
"Returns a Dynamic that delegates to this mapping" would be a clear explanation of a good implementation.
| @@ -220,6 +220,11 @@ public VmCollection.Builder<VmList> builder() { | |||
| return new Builder(); | |||
| } | |||
|
|
|||
| @TruffleBoundary | |||
| public static VmCollection.Builder<VmList> newBuilder() { | |||
There was a problem hiding this comment.
Existing code uses VmList.EMPTY.builder() instead.
Methods that "project out" of the type can be added to
ListingandMappingto avoid having to usetoList()/toMap(). Methods that result in aListing, resp.Mapping, are not included for performance and (lazy) semantics reasons, but methods likecontainsetc. can be defined straightforwardly.