Skip to content

Commit 9a4363a

Browse files
committed
Refactor Iterator implementation for better maintainability
Added helper methods to reduce code duplication: - getNextMethod(): Extract and validate next method - callNext(): Call next and validate result - isDone(): Check if iteration is complete - getValue(): Extract value from result object - closeIterator(): Close iterator via return() method Removed redundant inline comments that duplicated method names. Consolidated repetitive iterator protocol validation. No functional changes - purely code quality improvements.
1 parent 5cc1906 commit 9a4363a

File tree

1 file changed

+36
-49
lines changed

1 file changed

+36
-49
lines changed

rhino/src/main/java/org/mozilla/javascript/NativeES2025Iterator.java

Lines changed: 36 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -6,18 +6,12 @@
66

77
package org.mozilla.javascript;
88

9-
/**
10-
* Implementation of the ES2025 Iterator constructor and Iterator.prototype. This provides the
11-
* Iterator constructor function and static methods like Iterator.from.
12-
*/
9+
/** ES2025 Iterator constructor and helper methods. */
1310
public class NativeES2025Iterator extends ScriptableObject {
1411
private static final long serialVersionUID = 1L;
15-
1612
private static final String CLASS_NAME = "Iterator";
1713

1814
static void init(Context cx, ScriptableObject scope, boolean sealed) {
19-
// Create Lambda constructor
20-
// Iterator constructor throws TypeError when called
2115
LambdaConstructor constructor =
2216
new LambdaConstructor(
2317
scope,
@@ -26,22 +20,18 @@ static void init(Context cx, ScriptableObject scope, boolean sealed) {
2620
LambdaConstructor.CONSTRUCTOR_FUNCTION,
2721
NativeES2025Iterator::constructor);
2822

29-
// Define static methods on constructor
3023
constructor.defineConstructorMethod(
3124
scope, "from", 1, NativeES2025Iterator::js_from, DONTENUM);
3225
constructor.defineConstructorMethod(
3326
scope, "concat", 0, NativeES2025Iterator::js_concat, DONTENUM);
3427

35-
// Define prototype methods
3628
constructor.definePrototypeMethod(
3729
scope,
3830
SymbolKey.ITERATOR,
3931
0,
4032
NativeES2025Iterator::js_iterator,
4133
DONTENUM,
4234
DONTENUM);
43-
44-
// Iterator helper methods
4535
constructor.definePrototypeMethod(
4636
scope, "map", 1, NativeES2025Iterator::js_map, DONTENUM, DONTENUM);
4737
constructor.definePrototypeMethod(
@@ -65,7 +55,6 @@ static void init(Context cx, ScriptableObject scope, boolean sealed) {
6555
constructor.definePrototypeMethod(
6656
scope, "find", 1, NativeES2025Iterator::js_find, DONTENUM, DONTENUM);
6757

68-
// Define Symbol.toStringTag as a value property on prototype
6958
constructor.definePrototypeProperty(
7059
SymbolKey.TO_STRING_TAG, CLASS_NAME, DONTENUM | READONLY);
7160

@@ -84,46 +73,33 @@ public String getClassName() {
8473
return CLASS_NAME;
8574
}
8675

87-
// Lambda-based method implementations
88-
8976
private static Scriptable constructor(Context cx, Scriptable scope, Object[] args) {
90-
// ES2025 Iterator constructor throws TypeError when called
9177
throw ScriptRuntime.typeError("Iterator is not a constructor");
9278
}
9379

9480
private static Object js_iterator(
9581
Context cx, Scriptable scope, Scriptable thisObj, Object[] args) {
96-
// Symbol.iterator returns this
9782
return thisObj;
9883
}
9984

10085
private static Object js_from(Context cx, Scriptable scope, Scriptable thisObj, Object[] args) {
101-
// Iterator.from(item) implementation
102-
// If item is already an iterator with proper prototype, return it
103-
// Otherwise wrap it in an iterator
104-
10586
Object item = args.length > 0 ? args[0] : Undefined.instance;
10687

10788
if (item == null || item == Undefined.instance) {
10889
throw ScriptRuntime.typeError("Cannot convert undefined or null to iterator");
10990
}
11091

111-
// Check if it's already an iterator that inherits from Iterator.prototype
11292
if (item instanceof ES2025IteratorPrototype) {
11393
return item;
11494
}
11595

116-
// Check for Symbol.iterator method
11796
if (item instanceof Scriptable) {
11897
Scriptable scriptable = (Scriptable) item;
11998
Object iteratorMethod = ScriptableObject.getProperty(scriptable, SymbolKey.ITERATOR);
12099

121100
if (iteratorMethod != Scriptable.NOT_FOUND && iteratorMethod instanceof Callable) {
122-
// Call @@iterator to get iterator
123101
Callable callable = (Callable) iteratorMethod;
124102
Object iterator = callable.call(cx, scope, scriptable, ScriptRuntime.emptyArgs);
125-
126-
// Wrap the iterator to inherit from Iterator.prototype
127103
return new WrappedIterator(cx, scope, iterator);
128104
}
129105
}
@@ -133,22 +109,17 @@ private static Object js_from(Context cx, Scriptable scope, Scriptable thisObj,
133109

134110
private static Object js_concat(
135111
Context cx, Scriptable scope, Scriptable thisObj, Object[] args) {
136-
// Iterator.concat(...items) implementation
137-
// Validate and collect iterables
138112
Scriptable[] iterables = new Scriptable[args.length];
139113
Callable[] iteratorMethods = new Callable[args.length];
140114

141115
for (int i = 0; i < args.length; i++) {
142116
Object item = args[i];
143117

144-
// Each item must be an object
145118
if (!(item instanceof Scriptable)) {
146119
throw ScriptRuntime.typeError("Iterator.concat requires iterable objects");
147120
}
148121

149122
Scriptable itemObj = (Scriptable) item;
150-
151-
// Get the Symbol.iterator method
152123
Object iteratorMethod = ScriptableObject.getProperty(itemObj, SymbolKey.ITERATOR);
153124

154125
if (!(iteratorMethod instanceof Callable)) {
@@ -163,35 +134,51 @@ private static Object js_concat(
163134
return new ConcatIterator(cx, scope, iterables, iteratorMethods);
164135
}
165136

166-
// Iterator.prototype.toArray()
167-
private static Object js_toArray(
168-
Context cx, Scriptable scope, Scriptable thisObj, Object[] args) {
169-
// Get the iterator's next method
170-
Object next = ScriptableObject.getProperty(thisObj, "next");
137+
private static Callable getNextMethod(Scriptable iterator) {
138+
Object next = ScriptableObject.getProperty(iterator, "next");
171139
if (!(next instanceof Callable)) {
172140
throw ScriptRuntime.typeError("Iterator must have a next method");
173141
}
174-
Callable nextMethod = (Callable) next;
142+
return (Callable) next;
143+
}
144+
145+
private static Scriptable callNext(
146+
Context cx, Scriptable scope, Scriptable thisObj, Callable nextMethod) {
147+
Object result = nextMethod.call(cx, scope, thisObj, ScriptRuntime.emptyArgs);
148+
if (!(result instanceof Scriptable)) {
149+
throw ScriptRuntime.typeError("Iterator result must be an object");
150+
}
151+
return (Scriptable) result;
152+
}
153+
154+
private static boolean isDone(Scriptable result) {
155+
Object done = ScriptableObject.getProperty(result, "done");
156+
return ScriptRuntime.toBoolean(done);
157+
}
175158

176-
// Create array to collect values
159+
private static Object getValue(Scriptable result) {
160+
return ScriptableObject.getProperty(result, "value");
161+
}
162+
163+
private static void closeIterator(Context cx, Scriptable scope, Scriptable iterator) {
164+
Object returnMethod = ScriptableObject.getProperty(iterator, "return");
165+
if (returnMethod instanceof Callable) {
166+
((Callable) returnMethod).call(cx, scope, iterator, ScriptRuntime.emptyArgs);
167+
}
168+
}
169+
170+
private static Object js_toArray(
171+
Context cx, Scriptable scope, Scriptable thisObj, Object[] args) {
172+
Callable nextMethod = getNextMethod(thisObj);
177173
Scriptable array = cx.newArray(scope, 0);
178174
int index = 0;
179175

180-
// Iterate and collect all values
181176
while (true) {
182-
Object result = nextMethod.call(cx, scope, thisObj, ScriptRuntime.emptyArgs);
183-
if (!(result instanceof Scriptable)) {
184-
throw ScriptRuntime.typeError("Iterator result must be an object");
185-
}
186-
Scriptable resultObj = (Scriptable) result;
187-
188-
Object done = ScriptableObject.getProperty(resultObj, "done");
189-
if (ScriptRuntime.toBoolean(done)) {
177+
Scriptable result = callNext(cx, scope, thisObj, nextMethod);
178+
if (isDone(result)) {
190179
break;
191180
}
192-
193-
Object value = ScriptableObject.getProperty(resultObj, "value");
194-
array.put(index++, array, value);
181+
array.put(index++, array, getValue(result));
195182
}
196183

197184
return array;

0 commit comments

Comments
 (0)