[MATH-367] AbstractRealVector.sparseIterator fails when vector has exactly one non-zero entry

• Type: Bug
• Status: Closed
• Priority: Minor
• Resolution: Fixed
• Affects Version/s: 2.1, 2.2
• Fix Version/s: 2.2
• Assignee: William Barker
• Reporter: Albert Huang
• Created: 22/Apr/10 18:31
• Updated: 23/Mar/11 20:03
• Resolved: 10/May/10 01:17

Description

The following program:
===
import java.util.Iterator;
import org.apache.commons.math.linear.*;

public class SparseIteratorTester
{
public static void main(String[] args) {
double vdata[] =

{ 0.0, 1.0, 0.0 }

;
RealVector v = new ArrayRealVector(vdata);
Iterator<RealVector.Entry> iter = v.sparseIterator();
while(iter.hasNext())

{ RealVector.Entry entry = iter.next(); System.out.printf("%d: %f\n", entry.getIndex(), entry.getValue()); }

}
}
===
generates this output:

1: 1.000000
Exception in thread "main" java.lang.ArrayIndexOutOfBoundsException: -1
at org.apache.commons.math.linear.ArrayRealVector.getEntry(ArrayRealVector.java:995)
at org.apache.commons.math.linear.AbstractRealVector$EntryImpl.getValue(AbstractRealVector.java:850)
at test.SparseIteratorTester.main(SparseIteratorTester.java:13)
===

This patch fixes it, and simplifies AbstractRealVector.SparseEntryIterator (sorry, i don't see any form entry for attaching a file)
===
Index: src/main/java/org/apache/commons/math/linear/AbstractRealVector.java
===================================================================
— src/main/java/org/apache/commons/math/linear/AbstractRealVector.java (revision 936985)
+++ src/main/java/org/apache/commons/math/linear/AbstractRealVector.java (working copy)
@@ -18,6 +18,7 @@
package org.apache.commons.math.linear;

import java.util.Iterator;
+import java.util.NoSuchElementException;

import org.apache.commons.math.FunctionEvaluationException;
import org.apache.commons.math.MathRuntimeException;
@@ -875,40 +876,25 @@
/** Dimension of the vector. */
private final int dim;

• /** Temporary entry (reused on each call to {@link #next()}

  . */

• private EntryImpl tmp = new EntryImpl();
  -
• /** Current entry. */
  + /** Last entry returned by #next(). */
  private EntryImpl current;

• /** Next entry. */
  + /** Next entry for #next() to return. */
  private EntryImpl next;

/** Simple constructor. */
protected SparseEntryIterator() {
dim = getDimension();
current = new EntryImpl();

• if (current.getValue() == 0) { - advance(current); - }
• if(current.getIndex() >= 0) { - // There is at least one non-zero entry - next = new EntryImpl(); - next.setIndex(current.getIndex()); + next = new EntryImpl(); + if(next.getValue() == 0) advance(next); - }

  else

  { - // The vector consists of only zero entries, so deny having a next - current = null; - }

  }

• /** Advance an entry up to the next non null one.
  + /** Advance an entry up to the next nonzero value.

• @param e entry to advance
  */
  protected void advance(EntryImpl e) {

• if (e == null) { - return; - }

  do

  { e.setIndex(e.getIndex() + 1); }

  while (e.getIndex() < dim && e.getValue() == 0);
  @@ -919,22 +905,17 @@

/**

{@inheritDoc} */
public boolean hasNext() { - return current != null; + return next.getIndex() >= 0; }

/** {@inheritDoc}

*/
public Entry next() {

• tmp.setIndex(current.getIndex());
• if (next != null) {
• current.setIndex(next.getIndex());
• advance(next);
• if (next.getIndex() < 0) { - next = null; - }
• } else { - current = null; - }
• return tmp;
  + int index = next.getIndex();
  + if(index < 0)
  + throw new NoSuchElementException();
  + current.setIndex(index);
  + advance(next);
  + return current;
  }

/**

{@inheritDoc}

*/