You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by GitBox <gi...@apache.org> on 2022/09/08 19:24:20 UTC
[GitHub] [commons-numbers] aherbert commented on a diff in pull request #124: Numbers 186.complex list interleaved
aherbert commented on code in PR #124:
URL: https://github.com/apache/commons-numbers/pull/124#discussion_r966333894
##########
commons-numbers-complex-arrays/src/main/java/org/apache/commons/numbers/complex/arrays/ComplexList.java:
##########
@@ -28,23 +28,9 @@
import java.util.Objects;
/**
- * Resizable-double array implementation of the List interface. Implements all optional list operations,
- * and permits all complex numbers. In addition to implementing the List interface,
- * this class provides methods to manipulate the size of the array that is used internally to store the list.
- *
- * <p>Each ComplexList instance has a capacity. The capacity is half the size of the double array used to store the complex numbers
- * in the list. As complex numbers are added to an ComplexList, its capacity grows automatically.
- * The complex number is stored using an interleaved format and so the maximum number of complex numbers that may be added is
- * approximately 2<sup>30</sup>. This is half the maximum capacity of java.util.ArrayList.
- * The memory usage is more efficient than using a List of Complex objects as the underlying numbers are not stored
- * using instances of Complex.</p>
- *
- * <p>An application can increase the capacity of an ComplexList instance before adding a large number of complex numbers
- * using the ensureCapacity operation. This may reduce the amount of incremental reallocation.</p>
- *
- * <p>This list does not support {@code null} Complex objects.
+ * This is an abstract class that implements all the common data and operations for a ComplexList.
*/
-public class ComplexList extends AbstractList<Complex> {
+public abstract class ComplexList extends AbstractList<Complex> {
Review Comment:
Prevent extension of this class outside the inner classes by adding a private constructor. It may have to be package-private to pass the build checks (e.g. PMD).
##########
commons-numbers-complex-arrays/src/main/java/org/apache/commons/numbers/complex/arrays/ComplexList.java:
##########
@@ -395,46 +229,322 @@ private String outOfBoundsMsg(int index) {
}
/**
- * Replaces each complex number of the list with the result of applying the operator to that complex number.
+ * Constructs an empty interleaved list which can store up to the specified capacity without a memory reallocation.
*
- * @param operator The operator to apply to each complex number.
- * @throws NullPointerException if the specified operator is null.
+ * @param capacity Capacity of interleaved list.
+ * @return ComplexList object.
+ * @throws IllegalArgumentException if the {@code capacity} is greater than {@code MAX_CAPACITY}.
*/
- public void replaceAll(ComplexUnaryOperator<Void> operator) {
- Objects.requireNonNull(operator);
- final double[] parts = this.realAndImagParts;
- final int m = size;
- final int expectedModCount = modCount;
- for (int i = 0; i < m; i++) {
- final int index = i << 1;
- operator.apply(parts[index], parts[index + 1], (x, y) -> {
- parts[index] = x;
- parts[index + 1] = y;
- return null;
- });
- }
- // check for comodification
- if (modCount != expectedModCount) {
- throw new ConcurrentModificationException();
- }
- modCount++;
+ public static ComplexList interleaved(int capacity) {
+ return new ComplexInterleavedList(capacity);
}
/**
- * Performs the given action for each complex number of the list until all complex numbers
- * have been processed or the action throws an exception. Unless otherwise specified by the
- * implementing class, actions are performed in the order of iteration.
+ * Constructs an empty interleaved list.
*
- * @param action The action to apply to each complex number.
- * @throws NullPointerException if the specified action is null.
+ * @return ComplexList object.
+ */
+ public static ComplexList interleaved() {
+ return new ComplexInterleavedList();
+ }
+
+ /**
+ * Constructs an interleaved list using the specified double array.
+ *
+ * @param data Specified backing double array.
+ * @return ComplexList object.
+ * @throws IllegalArgumentException if the specified double array doesn't have an even amount of doubles.
*/
- public void forEach(ComplexConsumer action) {
- Objects.requireNonNull(action);
- final double[] parts = this.realAndImagParts;
- final int m = size;
- for (int i = 0; i < m; i++) {
- final int index = i << 1;
- action.accept(parts[index], parts[index + 1]);
+ public static ComplexList from(double[] data) {
+ return new ComplexInterleavedList(data);
+ }
+
+ /**
+ * Resizable-double array implementation of the List interface. Implements all optional list operations,
+ * and permits all complex numbers. In addition to implementing the List interface,
+ * this class provides methods to manipulate the size of the array that is used internally to store the list.
+ *
+ * <p>Each ComplexInterleavedList instance has a capacity. The capacity is half the size of the double array used to store the complex numbers
+ * in the list. As complex numbers are added to an ComplexInterleavedList, its capacity grows automatically.
+ * The complex number is stored using an interleaved format and so the maximum number of complex numbers that may be added is
+ * approximately 2<sup>30</sup>. This is half the maximum capacity of java.util.ArrayList.
+ * The memory usage is more efficient than using a List of Complex objects as the underlying numbers are not stored
+ * using instances of Complex.</p>
+ *
+ * <p>An application can increase the capacity of an ComplexInterleavedList instance before adding a large number of complex numbers
+ * using the ensureCapacity operation. This may reduce the amount of incremental reallocation.</p>
+ *
+ * <p>This list does not support {@code null} Complex objects.
+ */
+ private static class ComplexInterleavedList extends ComplexList {
+ /**
+ * Storage for the complex numbers.
+ * Data is stored in an interleaved format using (real, imaginary) pairs.
+ */
+ private double[] realAndImagParts;
+
+ /**
+ * Constructs an empty list which can store up to the specified capacity without a memory reallocation.
+ *
+ * @param capacity Capacity of list.
+ * @throws IllegalArgumentException if the {@code capacity} is greater than {@code MAX_CAPACITY}.
+ */
+ ComplexInterleavedList(int capacity) {
+ if (capacity > MAX_CAPACITY) {
+ throw new IllegalArgumentException(String.format(OOM_ERROR_STRING, capacity));
+ }
+ final int arrayLength = Math.max(DEFAULT_CAPACITY, capacity) * 2;
+ realAndImagParts = new double[arrayLength];
+ }
+
+ /**
+ * Constructs an empty list.
+ */
+ ComplexInterleavedList() {
+ realAndImagParts = new double[DEFAULT_CAPACITY * 2];
+ }
+
+ /**
+ * Constructs an interleaved list using the specified double array.
+ *
+ * @param fromArray Specified backing double array.
+ * @throws IllegalArgumentException if the specified double array doesn't have an even amount of doubles.
+ */
+ ComplexInterleavedList(double[] fromArray) {
+ if (fromArray.length % 2 != 0) {
Review Comment:
if ((fromArray.length & 1) != 0)
##########
commons-numbers-complex-arrays/src/test/java/org/apache/commons/numbers/complex/arrays/ComplexListTest.java:
##########
@@ -105,13 +116,31 @@ void testAddAndAddAll() {
assertListOperation(list -> list.addAll(list1), l5, l6);
//Test for adding an empty list to an empty list
- ComplexList list = new ComplexList();
+ ComplexList list = ComplexList.interleaved();
assertListOperation(l -> {
l.addAll(list);
return l.addAll(0, list);
});
}
+ @Test
+ void testSetAddAndAddAllNullPointerException() {
+ ComplexList list1 = generateComplexList(3);
+ Assertions.assertThrows(NullPointerException.class, () -> list1.add(null));
+ Assertions.assertThrows(NullPointerException.class, () -> list1.add(0, null));
+ Assertions.assertThrows(NullPointerException.class, () -> list1.set(1, null));
+
+ List<Complex> list2 = generateList(3);
+ for (int i = 0; i < list2.size(); i++) {
+ Complex val = list2.get(i);
+ if (val != null) {
+ list2.set(i, null);
+ }
+ }
+ Assertions.assertThrows(NullPointerException.class, () -> list1.addAll(list2));
Review Comment:
Check that list1 has the same size after the NPE is thrown, i.e. it is unchanged.
You could create a copy of the list for reference and use assertEquals to compare the before and after.
##########
commons-numbers-complex-arrays/src/main/java/org/apache/commons/numbers/complex/arrays/ComplexList.java:
##########
@@ -28,23 +28,9 @@
import java.util.Objects;
/**
- * Resizable-double array implementation of the List interface. Implements all optional list operations,
Review Comment:
Some of this javadoc still applies. The list provides efficient storage of complex numbers. It does not accept null complex objects.
##########
commons-numbers-complex-arrays/src/test/java/org/apache/commons/numbers/complex/arrays/ComplexListTest.java:
##########
@@ -105,13 +116,31 @@ void testAddAndAddAll() {
assertListOperation(list -> list.addAll(list1), l5, l6);
//Test for adding an empty list to an empty list
- ComplexList list = new ComplexList();
+ ComplexList list = ComplexList.interleaved();
assertListOperation(l -> {
l.addAll(list);
return l.addAll(0, list);
});
}
+ @Test
+ void testSetAddAndAddAllNullPointerException() {
+ ComplexList list1 = generateComplexList(3);
+ Assertions.assertThrows(NullPointerException.class, () -> list1.add(null));
+ Assertions.assertThrows(NullPointerException.class, () -> list1.add(0, null));
+ Assertions.assertThrows(NullPointerException.class, () -> list1.set(1, null));
+
+ List<Complex> list2 = generateList(3);
+ for (int i = 0; i < list2.size(); i++) {
Review Comment:
This loop is setting all elements to null. Just set one of them:
`list2.set(1, null)`
##########
commons-numbers-complex-arrays/src/test/java/org/apache/commons/numbers/complex/arrays/ComplexListTest.java:
##########
@@ -37,6 +37,17 @@ public class ComplexListTest {
private static final int MAX_CAPACITY = (Integer.MAX_VALUE - 9) / 2;
+ @Test
+ void testFromArray() {
+ double[] fromArray1 = ThreadLocalRandom.current().doubles(6, -Math.PI, Math.PI).toArray();
Review Comment:
```
int size = 3;
... doubles(size * 2, ...
for (... ; i < size; ...
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org