Github user paplorinc commented on a diff in the pull request:
https://github.com/apache/groovy/pull/360#discussion_r69004132
--- Diff: src/main/groovy/lang/ObjectRange.java ---
@@ -368,149 +367,152 @@ public String inspect() {
* Also see containsWithinBounds.
*/
public boolean contains(Object value) {
- Iterator it = new StepIterator(this, 1);
- if (value == null) return false;
- while (it.hasNext()) {
- try {
- if (DefaultTypeTransformation.compareEqual(value,
it.next())) return true;
- } catch (ClassCastException e) {
- return false;
+ if (value == null)
+ return false;
+
+ for (Iterator it = iterator(); it.hasNext(); ) {
+ if (compareEqual(value, it.next())) {
+ return true;
}
}
return false;
}
+ @Override
public void step(int step, Closure closure) {
- if (step == 0 && compareTo(from, to) == 0) {
+ step((Number) step, closure);
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ @Override
+ public void step(Number step, Closure closure) {
+ if (compareEqual(step, 0) && compareTo(from, to) == 0)
return; // from == to and step == 0, nothing to do, so return
- }
- StepIterator iter = new StepIterator(this, step);
- while (iter.hasNext()) {
- closure.call(iter.next());
- }
+
+ for (StepIterator it = new StepIterator(this, step); it.hasNext();
)
+ closure.call(it.next());
}
/**
* {@inheritDoc}
*/
public Iterator iterator() {
- // non thread-safe iterator
- final Iterator innerIterator = new StepIterator(this, 1);
- Iterator safeIterator = new Iterator() {
+ return new Iterator() {
+ final Iterator innerIterator = new
StepIterator(ObjectRange.this, 1);
+
@Override
- public synchronized boolean hasNext() {
+ public boolean hasNext() {
return innerIterator.hasNext();
}
@Override
- public synchronized Object next() {
+ public Object next() {
return innerIterator.next();
}
@Override
public void remove() {
innerIterator.remove();
}
};
- return safeIterator;
}
+
/**
* convenience class to serve in other methods.
* It's not thread-safe, and lazily produces the next element only on
calls of hasNext() or next()
*/
- private static class StepIterator implements Iterator {
- // actual step, can be +1 when desired step is -1 and direction is
from high to low
- private final int step;
+ static class StepIterator implements Iterator {
private final ObjectRange range;
- private int index = -1;
- private Comparable value;
- private boolean nextFetched = true;
- private StepIterator(ObjectRange range, final int desiredStep) {
- if (desiredStep == 0 && range.compareTo(range.getFrom(),
range.getTo()) != 0) {
+ private final Number step;
+ private final boolean isAscending;
+ private Comparable next;
+
+ StepIterator(ObjectRange range, Number desiredStep) {
+ if (compareEqual(desiredStep, 0) &&
compareNotEqual(range.getFrom(), range.getTo())) {
throw new GroovyRuntimeException("Infinite loop detected
due to step size of 0");
}
+
this.range = range;
- if (range.isReverse()) {
- this.step = -desiredStep;
+ if (compareLessThan(desiredStep, 0)) {
+ step = multiply(desiredStep, -1);
+ isAscending = range.isReverse();
} else {
- this.step = desiredStep;
- }
- if (this.step > 0) {
- value = range.getFrom();
- } else {
- value = range.getTo();
+ step = desiredStep;
+ isAscending = !range.isReverse();
}
+ next = isAscending ? range.getFrom() : range.getTo();
}
+
public void remove() {
- this.range.remove(index);
+ throw new UnsupportedOperationException();
}
+
public Object next() {
- // not thread safe
- if (!nextFetched) {
- value = peek();
- nextFetched = true;
- }
- nextFetched = false;
- index++;
- return value;
+ if (!hasNext()) return null;
--- End diff --
apparently this was a separate requirement - the reason for the original
test failures
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
