This is an automated email from the ASF dual-hosted git repository.
desruisseaux pushed a commit to branch geoapi-4.0
in repository https://gitbox.apache.org/repos/asf/sis.git
commit 359caffd38a46c91bcf0273555e7763b27584bee
Author: Martin Desruisseaux <martin.desruisse...@geomatys.com>
AuthorDate: Sat Feb 26 16:40:24 2022 +0100
Values returned by `ArrayVector.get(int)` need to be instance of more
predictable classes.
This is needed by the netCDF reader of features, which expects instances of
specific types.
---
.../org/apache/sis/feature/AbstractFeature.java | 2 +-
.../main/java/org/apache/sis/math/ArrayVector.java | 24 +++-----
.../src/main/java/org/apache/sis/math/Vector.java | 12 +++-
.../test/java/org/apache/sis/math/VectorTest.java | 69 +++++++++++++---------
4 files changed, 61 insertions(+), 46 deletions(-)
diff --git
a/core/sis-feature/src/main/java/org/apache/sis/feature/AbstractFeature.java
b/core/sis-feature/src/main/java/org/apache/sis/feature/AbstractFeature.java
index dcc2fd6..3afed12 100644
--- a/core/sis-feature/src/main/java/org/apache/sis/feature/AbstractFeature.java
+++ b/core/sis-feature/src/main/java/org/apache/sis/feature/AbstractFeature.java
@@ -510,7 +510,7 @@ public abstract class AbstractFeature implements Feature,
Serializable {
} while ((element = it.next()) == null ||
base.isInstance(element));
// Found an illegal value. Exeption is thrown below.
}
- throw new ClassCastException(illegalValueClass(pt, base,
element)); // 'element' can not be null here.
+ throw new ClassCastException(illegalValueClass(pt, base,
element)); // `element` can not be null here.
}
}
((Attribute) attribute).setValue(value);
diff --git
a/core/sis-utility/src/main/java/org/apache/sis/math/ArrayVector.java
b/core/sis-utility/src/main/java/org/apache/sis/math/ArrayVector.java
index 25c2aea..1b5c65a 100644
--- a/core/sis-utility/src/main/java/org/apache/sis/math/ArrayVector.java
+++ b/core/sis-utility/src/main/java/org/apache/sis/math/ArrayVector.java
@@ -42,7 +42,7 @@ import org.apache.sis.measure.NumberRange;
* so changes in the underlying array is reflected in this vector and
vice-versa.
*
* @author Martin Desruisseaux (MPO, Geomatys)
- * @version 1.1
+ * @version 1.2
* @since 0.8
* @module
*/
@@ -1039,11 +1039,9 @@ abstract class ArrayVector<E extends Number> extends
Vector implements CheckedCo
throw new
ArithmeticException(Errors.format(Errors.Keys.IntegerOverflow_1, Integer.SIZE));
}
- /** Uses a larger type if the value exceed integer capacity. */
+ /** Unconditionally uses a larger type since the value may exceed
integer capacity. */
@Override public Number get(int index) {
- final int value = super.intValue(index);
- if (value >= 0) return value;
- return Integer.toUnsignedLong(value);
+ return longValue(index);
}
/** Verifies that the given value can be stored as an unsigned
integer. */
@@ -1095,11 +1093,9 @@ abstract class ArrayVector<E extends Number> extends
Vector implements CheckedCo
throw new
ArithmeticException(Errors.format(Errors.Keys.IntegerOverflow_1, Short.SIZE));
}
- /** Uses a larger type if the value exceed short integer capacity. */
+ /** Unconditionally uses a larger type since the value may exceed
short capacity. */
@Override public Number get(int index) {
- final short value = super.shortValue(index);
- if (value >= 0) return value;
- return Short.toUnsignedInt(value);
+ return intValue(index);
}
/** Verifies that the given value can be stored as an unsigned
integer. */
@@ -1143,8 +1139,8 @@ abstract class ArrayVector<E extends Number> extends
Vector implements CheckedCo
@Override public boolean isUnsigned() {return true;}
@Override public double doubleValue(int index) {return
intValue(index);}
@Override public float floatValue(int index) {return
intValue(index);}
- @Override public long longValue(int index) {return
Byte.toUnsignedLong (super.byteValue(index));}
- @Override public int intValue(int index) {return
Byte.toUnsignedInt (super.byteValue(index));}
+ @Override public long longValue(int index) {return
Byte.toUnsignedLong(super.byteValue(index));}
+ @Override public int intValue(int index) {return
Byte.toUnsignedInt (super.byteValue(index));}
@Override public short shortValue(int index) {return (short)
intValue(index);}
@Override public byte byteValue(int index) {
final byte value = super.byteValue(index);
@@ -1152,11 +1148,9 @@ abstract class ArrayVector<E extends Number> extends
Vector implements CheckedCo
throw new
ArithmeticException(Errors.format(Errors.Keys.IntegerOverflow_1, Byte.SIZE));
}
- /** Uses a larger type if the value exceed integer capacity. */
+ /** Unconditionally uses a larger type since the value may exceed byte
capacity. */
@Override public Number get(int index) {
- final byte value = super.byteValue(index);
- if (value >= 0) return value;
- return (short) Byte.toUnsignedInt(value);
+ return shortValue(index);
}
/** Verifies that the given value can be stored as an unsigned
integer. */
diff --git a/core/sis-utility/src/main/java/org/apache/sis/math/Vector.java
b/core/sis-utility/src/main/java/org/apache/sis/math/Vector.java
index 97f4b97..90d0cca 100644
--- a/core/sis-utility/src/main/java/org/apache/sis/math/Vector.java
+++ b/core/sis-utility/src/main/java/org/apache/sis/math/Vector.java
@@ -248,7 +248,7 @@ public abstract class Vector extends AbstractList<Number>
implements RandomAcces
*
* <ul>
* <li>If this vector {@linkplain #isUnsigned() is unsigned}, then the
values returned by {@code get(int)}
- * may be instances of a type wider than the type used by this
vector for storing the values.</li>
+ * will be instances of a type wider than the type used by this
vector for storing the values.</li>
* <li>If this vector has been {@linkplain #createForDecimal(float[])
created for decimal numbers},
* then the values returned by {@code get(int)} will use
double-precision even if this vector
* stores the values as single-precision floating point numbers.</li>
@@ -518,8 +518,14 @@ public abstract class Vector extends AbstractList<Number>
implements RandomAcces
*
* <div class="note"><b>Example:</b>
* if {@link #getElementType()} returns {@code Byte.class} but {@link
#isUnsigned()} returns {@code true},
- * then this method may return instances of {@link Short} since that type
is the smallest Java primitive
- * type capable to hold byte values in the [0 … 255] range.</div>
+ * then this method will rather return instances of {@link Short} because
that type is the smallest Java
+ * primitive type capable to hold byte values in the [0 … 255] range. But
the elements are still stored
+ * internally as {@code byte}, and the vector can not accept values
outside the [0 … 255] range even if
+ * they are valid {@link Short} values.</div>
+ *
+ * The class of returned objects should be stable. For example this method
should not use different types
+ * for different range of values. This stability is recommended but not
guaranteed because {@code Vector}
+ * can also wrap arbitrary {@code Number[]} arrays.
*
* @param index the index in the [0 … {@linkplain #size() size}-1] range.
* @return the value at the given index (may be {@code null}).
diff --git a/core/sis-utility/src/test/java/org/apache/sis/math/VectorTest.java
b/core/sis-utility/src/test/java/org/apache/sis/math/VectorTest.java
index f2ed27c..c27581b 100644
--- a/core/sis-utility/src/test/java/org/apache/sis/math/VectorTest.java
+++ b/core/sis-utility/src/test/java/org/apache/sis/math/VectorTest.java
@@ -28,7 +28,7 @@ import static org.opengis.test.Assert.*;
* Tests the {@link Vector} class.
*
* @author Martin Desruisseaux (Geomatys)
- * @version 1.1
+ * @version 1.2
* @since 0.8
* @module
*/
@@ -69,26 +69,31 @@ public final strictfp class VectorTest extends TestCase {
* We use the {@code short} type for this test.
*/
@Test
+ @SuppressWarnings("UnnecessaryBoxing")
public void testShortArray() {
final short[] array = new short[400];
for (int i=0; i<array.length; i++) {
- array[i] = (short) ((i + 100) * 10);
+ array[i] = (short) ((i - 20) * 10);
}
vector = Vector.create(array, false);
assertTrue(vector instanceof ArrayVector);
assertSame(vector, Vector.create(vector, false));
assertEquals(array.length, vector.size());
assertEquals(Short.class, vector.getElementType());
-
- // Verifies element values.
+ /*
+ * Verify element values. The wrapper class shall be `Short`.
+ */
for (int i=0; i<array.length; i++) {
- assertEquals(array[i], vector.shortValue (i));
- assertEquals(array[i], vector.intValue (i));
- assertEquals(array[i], vector.floatValue (i), 0f);
- assertEquals(array[i], vector.doubleValue(i), STRICT);
+ final short expected = array[i];
+ assertEquals(expected, vector.shortValue (i));
+ assertEquals(expected, vector.intValue (i));
+ assertEquals(expected, vector.floatValue (i), 0f);
+ assertEquals(expected, vector.doubleValue(i), STRICT);
+ assertEquals(Short.valueOf(expected), vector.get(i));
}
-
- // Tests exception.
+ /*
+ * Test exception for invalid index and for invalid narrowing cast.
+ */
try {
vector.floatValue(array.length);
fail("Expected an IndexOutOfBoundsException");
@@ -97,15 +102,15 @@ public final strictfp class VectorTest extends TestCase {
}
try {
vector.byteValue(0);
- fail("Expected a ArithmeticException");
+ fail("Expected an ArithmeticException");
} catch (ArithmeticException e) {
// This is the expected exception.
final String message = e.getMessage();
- assertTrue(message, message.contains("1000"));
assertTrue(message, message.contains("byte"));
}
-
- // Tests subvector in the range [100:2:298].
+ /*
+ * Test subvector in the range [100:2:298].
+ */
vector = vector.subSampling(100, 2, 100);
assertEquals(100, vector.size());
for (int i=0; i<100; i++) {
@@ -117,8 +122,10 @@ public final strictfp class VectorTest extends TestCase {
} catch (IndexOutOfBoundsException e) {
// This is the expected exception.
}
-
- // Tests subvector at specific indexes.
+ /*
+ * Test subvector at specific indices. The indices of picked values
below and indices
+ * in the sub-vector tested above. They may to the original array by
`index*2 + 100`.
+ */
vector = vector.pick(10, 20, 25);
assertEquals(3, vector.size());
assertEquals(array[120], vector.shortValue(0));
@@ -130,6 +137,7 @@ public final strictfp class VectorTest extends TestCase {
* Tests {@link ArrayVector} backed by an array of primitive type handled
as unsigned values.
*/
@Test
+ @SuppressWarnings("UnnecessaryBoxing")
public void testUnsignedByteArray() {
final byte[] array = new byte[100];
for (int i=0; i<array.length; i++) {
@@ -140,27 +148,34 @@ public final strictfp class VectorTest extends TestCase {
assertSame(vector, Vector.create(vector, true));
assertEquals(array.length, vector.size());
assertEquals(Byte.class, vector.getElementType());
-
- // Verifies element values.
+ /*
+ * Verify element values. Bytes shall be casted to shorts in order to
handle unsigned values.
+ * The widening casts should be unconditional, even for positive
values that could fit in a byte,
+ * because some codes using `Vector` expect a stable type (for example
`NetcdfStore` which copies
+ * vector values into feature properties).
+ */
for (int i=0; i<array.length; i++) {
final int expected = Byte.toUnsignedInt(array[i]);
assertEquals(expected, vector.shortValue (i));
assertEquals(expected, vector.intValue (i));
assertEquals(expected, vector.floatValue (i), 0f);
assertEquals(expected, vector.doubleValue(i), STRICT);
+ assertEquals(Short.valueOf((short) expected), vector.get(i));
// See above comment.
}
-
- // Tests exception.
- assertEquals(106, vector.byteValue(50));
+ /*
+ * Test exception for invalid narrowing cast.
+ */
+ assertEquals(106, vector.byteValue (50));
assertEquals(146, vector.shortValue(70));
try {
vector.byteValue(70);
- fail("Expected a ArithmeticException");
+ fail("Expected an ArithmeticException");
} catch (ArithmeticException e) {
// This is the expected exception.
}
-
- // Test writing.
+ /*
+ * Test writing.
+ */
vector.set(70, (short) 200);
assertEquals(200, vector.shortValue(70));
}
@@ -203,7 +218,7 @@ public final strictfp class VectorTest extends TestCase {
assertEquals(array.length, vector.size());
assertEquals(Double.class, vector.getElementType());
/*
- * Tests element values.
+ * Test element values.
*/
for (int i=0; i<array.length; i++) {
assertEquals(array[i], vector.floatValue (i), 0f);
@@ -251,8 +266,8 @@ public final strictfp class VectorTest extends TestCase {
assertSame("Should be able to restitute the original vector.", v1,
v3.subList( 0, 40));
assertSame("Should be able to restitute the original vector.", v2,
v3.subList(40, 60));
/*
- * Tests concatenation of views at fixed indices. Should be
- * implemented as the concatenation of the indices arrays when
possible.
+ * Test concatenation of views at fixed indices.
+ * Should be implemented as the concatenation of the indices arrays
when possible.
*/
final Vector expected = v3.pick(10, 25, 30, 0, 35, 39);
v2 = v1.pick( 0, 35, 39);
