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/10 23:52:30 UTC
[GitHub] [commons-numbers] sumanth-rajkumar opened a new pull request, #125: NUMBERS-186 added complex non-interleaved list implementation
sumanth-rajkumar opened a new pull request, #125:
URL: https://github.com/apache/commons-numbers/pull/125
This PR gives a non-interleaved complex list implementation as a private sub-class in ComplexList.
Summary of changes:
ComplexInterleavedList - private sub-class with non-interleaved double arrays storage.
ComplexListTest - updates to calling ComplexList using ComplexNonInterleavedList.
--
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
[GitHub] [commons-numbers] aherbert commented on pull request #125: NUMBERS-186 added complex non-interleaved list implementation
Posted by GitBox <gi...@apache.org>.
aherbert commented on PR #125:
URL: https://github.com/apache/commons-numbers/pull/125#issuecomment-1243676603
Squashed and merged in commit 29bb423fe21497e1c75e09b8d4b5127748dad1bd
--
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
[GitHub] [commons-numbers] codecov-commenter commented on pull request #125: NUMBERS-186 added complex non-interleaved list implementation
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #125:
URL: https://github.com/apache/commons-numbers/pull/125#issuecomment-1242821720
# [Codecov](https://codecov.io/gh/apache/commons-numbers/pull/125?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#125](https://codecov.io/gh/apache/commons-numbers/pull/125?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (0398eaf) into [complex-gsoc-2022](https://codecov.io/gh/apache/commons-numbers/commit/b5e916467da1853af6ace6a44247c3bef96bc8c6?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b5e9164) will **not change** coverage.
> The diff coverage is `n/a`.
```diff
@@ Coverage Diff @@
## complex-gsoc-2022 #125 +/- ##
====================================================
Coverage 99.13% 99.13%
Complexity 1699 1699
====================================================
Files 65 65
Lines 4274 4274
Branches 836 836
====================================================
Hits 4237 4237
Misses 10 10
Partials 27 27
```
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
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
[GitHub] [commons-numbers] aherbert commented on a diff in pull request #125: NUMBERS-186 added complex non-interleaved list implementation
Posted by GitBox <gi...@apache.org>.
aherbert commented on code in PR #125:
URL: https://github.com/apache/commons-numbers/pull/125#discussion_r967773595
##########
commons-numbers-complex-arrays/src/test/java/org/apache/commons/numbers/complex/arrays/ComplexListTest.java:
##########
@@ -349,10 +561,14 @@ private static <T> void assertListOperation(Function<List<Complex>, T> operation
Assertions.assertEquals(l1, l2);
}
- private static <T> void assertListOperation(Function<List<Complex>, T> operation) {
+ private static <T> void assertListOperation1(Function<List<Complex>, T> operation) {
Review Comment:
`assertListOperationInterleaved`
Looking at the use cases for this method, I think it could be removed if tests are broken down and parameterized.
##########
commons-numbers-complex-arrays/src/main/java/org/apache/commons/numbers/complex/arrays/ComplexList.java:
##########
@@ -276,22 +322,11 @@ public static ComplexList from(double[] data) {
* 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>
+ * using the ensureCapacityInternal 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 {
- /**
- * The maximum size of array to allocate.
- * Ensuring max capacity is even with additional space for VM array headers.
- */
- private static final int MAX_ARRAY_SIZE = Integer.MAX_VALUE - 9;
Review Comment:
All of these except the MAX_CAPACITY are specified to this list
##########
commons-numbers-complex-arrays/src/test/java/org/apache/commons/numbers/complex/arrays/ComplexListTest.java:
##########
@@ -118,17 +140,81 @@ void testAddAndAddAll() {
//Test for adding an empty list to an empty list
ComplexList list = ComplexList.interleaved();
- assertListOperation(l -> {
+ assertListOperation1(l -> {
+ l.addAll(list);
+ return l.addAll(0, list);
+ });
+ }
+
+ @Test
+ void testAddAndAddAllForComplexNonInterleavedList() {
Review Comment:
Code duplication of the entire method for a new list is not maintainable. Since the list must pass the same test then you can do this using a parameterized test:
```Java
/**
* Generate a stream of arguments containing empty {@code Complex<List>} implementations.
*
* @return the stream of arguments
*/
static Stream<Arguments> listImplementations() {
return Stream.of(Arguments.of(ComplexList.interleaved()),
Arguments.of(ComplexList.nonInterleaved()));
}
@ParameterizedTest
@MethodSource({"listImplementations"})
void testAddAndAddAllList(List<Complex> l2) {
List<Complex> l1 = new ArrayList<>();
assertListOperation(list -> list.add(Complex.ofCartesian(1, 2)), l1, l2);
assertListOperation(list -> {
list.add(1, Complex.ofCartesian(10, 20));
return Boolean.TRUE;
}, l1, l2);
assertListOperation(list -> list.add(Complex.ofCartesian(13, 14)), l1, l2);
assertListOperation(list -> list.add(Complex.ofCartesian(15, 16)), l1, l2);
assertListOperation(list -> list.add(Complex.ofCartesian(17, 18)), l1, l2);
assertListOperation(list -> {
list.addAll(1, list);
return Boolean.TRUE;
}, l1, l2);
assertListOperation(list -> list.add(Complex.ofCartesian(19, 20)), l1, l2);
assertListOperation(list -> list.add(Complex.ofCartesian(21, 22)), l1, l2);
assertListOperation(list -> list.add(Complex.ofCartesian(23, 24)), l1, l2);
}
```
The test may have to be broken down into smaller tests as the parameter will be an empty list. If you wish to start with a list populated with e.g. 10 random Complex numbers then you can add a second static method to stream arguments with populated lists.
Note: You can create multiple parameters for the parameterized test if these require:
```Java
static Stream<Arguments> listImplementationsWithSize() {
return Stream.of(Arguments.of(ComplexList.interleaved(), 10),
Arguments.of(ComplexList.nonInterleaved(), 10));
}
@ParameterizedTest
@MethodSource({"listImplementationsWithSize"})
void testAddAndAddAllList(List<Complex> l2, int size) {
```
##########
commons-numbers-complex-arrays/src/test/java/org/apache/commons/numbers/complex/arrays/ComplexListTest.java:
##########
@@ -349,10 +561,14 @@ private static <T> void assertListOperation(Function<List<Complex>, T> operation
Assertions.assertEquals(l1, l2);
}
- private static <T> void assertListOperation(Function<List<Complex>, T> operation) {
+ private static <T> void assertListOperation1(Function<List<Complex>, T> operation) {
assertListOperation(operation, new ArrayList<>(), ComplexList.interleaved());
}
+ private static <T> void assertListOperation2(Function<List<Complex>, T> operation) {
Review Comment:
`assertListOperationNonInterleaved`
Looking at the use cases for this method, I think it could be removed if tests are broken down and parameterized.
##########
commons-numbers-complex-arrays/src/main/java/org/apache/commons/numbers/complex/arrays/ComplexList.java:
##########
@@ -37,6 +37,17 @@
*/
public abstract class ComplexList extends AbstractList<Complex> {
+ /**
+ * The maximum size of array to allocate.
+ * Ensuring max capacity is even with additional space for VM array headers.
+ */
+ protected static final int MAX_ARRAY_SIZE = Integer.MAX_VALUE - 9;
Review Comment:
These should not be here. The max capacity of the interleaved list is different from the non-interleaved list. Only the DEFAULT_CAPACITY should be shared.
##########
commons-numbers-complex-arrays/src/main/java/org/apache/commons/numbers/complex/arrays/ComplexList.java:
##########
@@ -566,4 +601,302 @@ public void forEach(ComplexConsumer action) {
}
}
}
+
+ /**
+ * Resizable-double arrays 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 arrays that are used internally to store the list.
+ *
+ * <p>Each ComplexNonInterleavedList instance has a capacity. The capacity is the size of the double arrays used to store the complex numbers
+ * in the list. As complex numbers are added to an ComplexNonInterleavedList, its capacity grows automatically.
+ * The complex number is stored using an non-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 ComplexNonInterleavedList instance before adding a large number of complex numbers
+ * using the ensureCapacityInternal operation. This may reduce the amount of incremental reallocation.</p>
+ *
+ * <p>This list does not support {@code null} Complex objects.
+ */
+ private static class ComplexNonInterleavedList extends ComplexList {
+
+ /**
+ * Storage for the real parts of complex numbers.
+ */
+ private double[] realParts;
+
+ /**
+ * Storage for the imaginary parts of complex numbers.
+ */
+ private double[] imaginaryParts;
+
+ /**
+ * 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}.
+ */
+ ComplexNonInterleavedList(int capacity) {
+ if (capacity > MAX_CAPACITY) {
+ throw new IllegalArgumentException(String.format(OOM_ERROR_STRING, capacity));
+ }
+ final int arrayLength = Math.max(DEFAULT_CAPACITY, capacity);
+ realParts = new double[arrayLength];
+ imaginaryParts = new double[arrayLength];
+ }
+
+ /**
+ * Constructs an empty list.
+ */
+ ComplexNonInterleavedList() {
+ realParts = new double[DEFAULT_CAPACITY];
+ imaginaryParts = new double[DEFAULT_CAPACITY];
+ }
+
+ /**
+ * Constructs a non-interleaved list using the specified double arrays.
+ * The data isn't defensively copied, the specified arrays is used in-place.
+ *
+ * @param fromReal Specified backing double array for real parts.
+ * @param fromImaginary Specified backing double array for imaginary parts.
+ * @throws IllegalArgumentException if the specified double arrays don't have the same amount of doubles.
+ */
+ ComplexNonInterleavedList(double[] fromReal, double[] fromImaginary) {
+ if (fromReal.length != fromImaginary.length) {
+ throw new IllegalArgumentException("Need the same amount of real and imaginary parts");
+ }
+ realParts = fromReal;
+ imaginaryParts = fromImaginary;
+ size = fromReal.length;
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public Complex get(int index) {
+ rangeCheck(index);
+ return Complex.ofCartesian(realParts[index], imaginaryParts[index]);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public double getReal(int index) {
+ rangeCheck(index);
+ return realParts[index];
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public double getImaginary(int index) {
+ rangeCheck(index);
+ return imaginaryParts[index];
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public Complex set(int index, Complex number) {
+ rangeCheck(index);
+ final Complex oldValue = Complex.ofCartesian(realParts[index], imaginaryParts[index]);
+ realParts[index] = number.getReal();
+ imaginaryParts[index] = number.getImaginary();
+ return oldValue;
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public void setReal(int index, double real) {
+ rangeCheck(index);
+ realParts[index] = real;
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public void setImaginary(int index, double imaginary) {
+ rangeCheck(index);
+ imaginaryParts[index] = imaginary;
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ double[] toArrayReal() {
+ final int length = size;
Review Comment:
return Arrays.copyOf(realParts, size);
##########
commons-numbers-complex-arrays/src/main/java/org/apache/commons/numbers/complex/arrays/ComplexList.java:
##########
@@ -566,4 +601,302 @@ public void forEach(ComplexConsumer action) {
}
}
}
+
+ /**
+ * Resizable-double arrays 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 arrays that are used internally to store the list.
+ *
+ * <p>Each ComplexNonInterleavedList instance has a capacity. The capacity is the size of the double arrays used to store the complex numbers
+ * in the list. As complex numbers are added to an ComplexNonInterleavedList, its capacity grows automatically.
+ * The complex number is stored using an non-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.
Review Comment:
2<sup>31</sup>, the same capacity as ArrayList
##########
commons-numbers-complex-arrays/src/main/java/org/apache/commons/numbers/complex/arrays/ComplexList.java:
##########
@@ -566,4 +601,302 @@ public void forEach(ComplexConsumer action) {
}
}
}
+
+ /**
+ * Resizable-double arrays 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 arrays that are used internally to store the list.
+ *
+ * <p>Each ComplexNonInterleavedList instance has a capacity. The capacity is the size of the double arrays used to store the complex numbers
+ * in the list. As complex numbers are added to an ComplexNonInterleavedList, its capacity grows automatically.
+ * The complex number is stored using an non-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 ComplexNonInterleavedList instance before adding a large number of complex numbers
+ * using the ensureCapacityInternal operation. This may reduce the amount of incremental reallocation.</p>
+ *
+ * <p>This list does not support {@code null} Complex objects.
+ */
+ private static class ComplexNonInterleavedList extends ComplexList {
+
+ /**
+ * Storage for the real parts of complex numbers.
+ */
+ private double[] realParts;
+
+ /**
+ * Storage for the imaginary parts of complex numbers.
+ */
+ private double[] imaginaryParts;
+
+ /**
+ * 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}.
+ */
+ ComplexNonInterleavedList(int capacity) {
+ if (capacity > MAX_CAPACITY) {
+ throw new IllegalArgumentException(String.format(OOM_ERROR_STRING, capacity));
+ }
+ final int arrayLength = Math.max(DEFAULT_CAPACITY, capacity);
+ realParts = new double[arrayLength];
+ imaginaryParts = new double[arrayLength];
+ }
+
+ /**
+ * Constructs an empty list.
+ */
+ ComplexNonInterleavedList() {
+ realParts = new double[DEFAULT_CAPACITY];
+ imaginaryParts = new double[DEFAULT_CAPACITY];
+ }
+
+ /**
+ * Constructs a non-interleaved list using the specified double arrays.
+ * The data isn't defensively copied, the specified arrays is used in-place.
+ *
+ * @param fromReal Specified backing double array for real parts.
+ * @param fromImaginary Specified backing double array for imaginary parts.
+ * @throws IllegalArgumentException if the specified double arrays don't have the same amount of doubles.
Review Comment:
Also throws a NullPointerException ...
##########
commons-numbers-complex-arrays/src/main/java/org/apache/commons/numbers/complex/arrays/ComplexList.java:
##########
@@ -566,4 +601,302 @@ public void forEach(ComplexConsumer action) {
}
}
}
+
+ /**
+ * Resizable-double arrays 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 arrays that are used internally to store the list.
+ *
+ * <p>Each ComplexNonInterleavedList instance has a capacity. The capacity is the size of the double arrays used to store the complex numbers
+ * in the list. As complex numbers are added to an ComplexNonInterleavedList, its capacity grows automatically.
+ * The complex number is stored using an non-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 ComplexNonInterleavedList instance before adding a large number of complex numbers
+ * using the ensureCapacityInternal operation. This may reduce the amount of incremental reallocation.</p>
+ *
+ * <p>This list does not support {@code null} Complex objects.
+ */
+ private static class ComplexNonInterleavedList extends ComplexList {
+
+ /**
+ * Storage for the real parts of complex numbers.
+ */
+ private double[] realParts;
+
+ /**
+ * Storage for the imaginary parts of complex numbers.
+ */
+ private double[] imaginaryParts;
+
+ /**
+ * 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}.
+ */
+ ComplexNonInterleavedList(int capacity) {
+ if (capacity > MAX_CAPACITY) {
+ throw new IllegalArgumentException(String.format(OOM_ERROR_STRING, capacity));
+ }
+ final int arrayLength = Math.max(DEFAULT_CAPACITY, capacity);
+ realParts = new double[arrayLength];
+ imaginaryParts = new double[arrayLength];
+ }
+
+ /**
+ * Constructs an empty list.
+ */
+ ComplexNonInterleavedList() {
+ realParts = new double[DEFAULT_CAPACITY];
+ imaginaryParts = new double[DEFAULT_CAPACITY];
+ }
+
+ /**
+ * Constructs a non-interleaved list using the specified double arrays.
+ * The data isn't defensively copied, the specified arrays is used in-place.
+ *
+ * @param fromReal Specified backing double array for real parts.
+ * @param fromImaginary Specified backing double array for imaginary parts.
+ * @throws IllegalArgumentException if the specified double arrays don't have the same amount of doubles.
+ */
+ ComplexNonInterleavedList(double[] fromReal, double[] fromImaginary) {
+ if (fromReal.length != fromImaginary.length) {
+ throw new IllegalArgumentException("Need the same amount of real and imaginary parts");
+ }
+ realParts = fromReal;
+ imaginaryParts = fromImaginary;
+ size = fromReal.length;
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public Complex get(int index) {
+ rangeCheck(index);
+ return Complex.ofCartesian(realParts[index], imaginaryParts[index]);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public double getReal(int index) {
+ rangeCheck(index);
+ return realParts[index];
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public double getImaginary(int index) {
+ rangeCheck(index);
+ return imaginaryParts[index];
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public Complex set(int index, Complex number) {
+ rangeCheck(index);
+ final Complex oldValue = Complex.ofCartesian(realParts[index], imaginaryParts[index]);
+ realParts[index] = number.getReal();
+ imaginaryParts[index] = number.getImaginary();
+ return oldValue;
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public void setReal(int index, double real) {
+ rangeCheck(index);
+ realParts[index] = real;
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public void setImaginary(int index, double imaginary) {
+ rangeCheck(index);
+ imaginaryParts[index] = imaginary;
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ double[] toArrayReal() {
+ final int length = size;
+ final double[] real = new double[length];
+ System.arraycopy(realParts, 0, real, 0, length);
+ return real;
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ double[] toArrayImaginary() {
+ final int length = size;
Review Comment:
return Arrays.copyOf(imaginaryParts, size);
##########
commons-numbers-complex-arrays/src/main/java/org/apache/commons/numbers/complex/arrays/ComplexList.java:
##########
@@ -566,4 +601,302 @@ public void forEach(ComplexConsumer action) {
}
}
}
+
+ /**
+ * Resizable-double arrays 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 arrays that are used internally to store the list.
+ *
+ * <p>Each ComplexNonInterleavedList instance has a capacity. The capacity is the size of the double arrays used to store the complex numbers
+ * in the list. As complex numbers are added to an ComplexNonInterleavedList, its capacity grows automatically.
+ * The complex number is stored using an non-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 ComplexNonInterleavedList instance before adding a large number of complex numbers
+ * using the ensureCapacityInternal operation. This may reduce the amount of incremental reallocation.</p>
+ *
+ * <p>This list does not support {@code null} Complex objects.
+ */
+ private static class ComplexNonInterleavedList extends ComplexList {
+
+ /**
+ * Storage for the real parts of complex numbers.
+ */
+ private double[] realParts;
+
+ /**
+ * Storage for the imaginary parts of complex numbers.
+ */
+ private double[] imaginaryParts;
+
+ /**
+ * 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}.
+ */
+ ComplexNonInterleavedList(int capacity) {
+ if (capacity > MAX_CAPACITY) {
+ throw new IllegalArgumentException(String.format(OOM_ERROR_STRING, capacity));
+ }
+ final int arrayLength = Math.max(DEFAULT_CAPACITY, capacity);
+ realParts = new double[arrayLength];
+ imaginaryParts = new double[arrayLength];
+ }
+
+ /**
+ * Constructs an empty list.
+ */
+ ComplexNonInterleavedList() {
+ realParts = new double[DEFAULT_CAPACITY];
+ imaginaryParts = new double[DEFAULT_CAPACITY];
+ }
+
+ /**
+ * Constructs a non-interleaved list using the specified double arrays.
+ * The data isn't defensively copied, the specified arrays is used in-place.
+ *
+ * @param fromReal Specified backing double array for real parts.
+ * @param fromImaginary Specified backing double array for imaginary parts.
+ * @throws IllegalArgumentException if the specified double arrays don't have the same amount of doubles.
+ */
+ ComplexNonInterleavedList(double[] fromReal, double[] fromImaginary) {
+ if (fromReal.length != fromImaginary.length) {
+ throw new IllegalArgumentException("Need the same amount of real and imaginary parts");
+ }
+ realParts = fromReal;
+ imaginaryParts = fromImaginary;
+ size = fromReal.length;
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public Complex get(int index) {
+ rangeCheck(index);
+ return Complex.ofCartesian(realParts[index], imaginaryParts[index]);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public double getReal(int index) {
+ rangeCheck(index);
+ return realParts[index];
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public double getImaginary(int index) {
+ rangeCheck(index);
+ return imaginaryParts[index];
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public Complex set(int index, Complex number) {
+ rangeCheck(index);
+ final Complex oldValue = Complex.ofCartesian(realParts[index], imaginaryParts[index]);
+ realParts[index] = number.getReal();
+ imaginaryParts[index] = number.getImaginary();
+ return oldValue;
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public void setReal(int index, double real) {
+ rangeCheck(index);
+ realParts[index] = real;
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public void setImaginary(int index, double imaginary) {
+ rangeCheck(index);
+ imaginaryParts[index] = imaginary;
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ double[] toArrayReal() {
+ final int length = size;
+ final double[] real = new double[length];
+ System.arraycopy(realParts, 0, real, 0, length);
+ return real;
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ double[] toArrayImaginary() {
+ final int length = size;
+ final double[] imaginary = new double[length];
+ System.arraycopy(imaginaryParts, 0, imaginary, 0, length);
+ return imaginary;
+ }
+
+ /**
+ * Increases the capacity of this ComplexNonInterleavedList instance, if necessary, to ensure that it can hold at
+ * least the amount of complex numbers specified by the minimum capacity argument.
+ *
+ * @param minCapacity Desired minimum capacity.
+ * @throws OutOfMemoryError if the {@code minCapacity} is greater than {@code MAX_ARRAY_SIZE}.
+ */
+ private void ensureCapacityInternal(int minCapacity) {
+ modCount++;
+ final long minArrayCapacity = Integer.toUnsignedLong(minCapacity);
+ if (minArrayCapacity > MAX_ARRAY_SIZE) {
+ throw new OutOfMemoryError(String.format(OOM_ERROR_STRING, minArrayCapacity));
+ }
+ final long oldArrayCapacity = realParts.length;
+ if (minArrayCapacity > oldArrayCapacity) {
+ long newArrayCapacity = oldArrayCapacity + (oldArrayCapacity >> 1);
+ // Round-odd up to even
Review Comment:
Rounding to even is not required here
--
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
[GitHub] [commons-numbers] aherbert closed pull request #125: NUMBERS-186 added complex non-interleaved list implementation
Posted by GitBox <gi...@apache.org>.
aherbert closed pull request #125: NUMBERS-186 added complex non-interleaved list implementation
URL: https://github.com/apache/commons-numbers/pull/125
--
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
[GitHub] [commons-numbers] aherbert commented on a diff in pull request #125: NUMBERS-186 added complex non-interleaved list implementation
Posted by GitBox <gi...@apache.org>.
aherbert commented on code in PR #125:
URL: https://github.com/apache/commons-numbers/pull/125#discussion_r967892456
##########
commons-numbers-complex-arrays/src/test/java/org/apache/commons/numbers/complex/arrays/ComplexListTest.java:
##########
@@ -41,6 +42,62 @@ public class ComplexListTest {
private static final int MAX_CAPACITY_INTERLEAVED = (Integer.MAX_VALUE - 9) / 2;
private static final int MAX_CAPACITY_NON_INTERLEAVED = Integer.MAX_VALUE - 9;
+ /**
+ * Generate a stream of arguments containing empty {@code Complex<List>} implementations.
+ *
+ * @return the stream of arguments
+ */
+ static Stream<Arguments> listImplementations() {
+ return Stream.of(Arguments.of(ComplexList.interleaved()),
+ Arguments.of(ComplexList.nonInterleaved()));
Review Comment:
Indent so that `Arguments` is aligned to the same word in the line above
##########
commons-numbers-complex-arrays/src/main/java/org/apache/commons/numbers/complex/arrays/ComplexList.java:
##########
@@ -566,4 +604,299 @@ public void forEach(ComplexConsumer action) {
}
}
}
+
+ /**
+ * Resizable-double arrays 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 arrays that are used internally to store the list.
+ *
+ * <p>Each ComplexNonInterleavedList instance has a capacity. The capacity is the size of the double arrays used to store the complex numbers
+ * in the list. As complex numbers are added to an ComplexNonInterleavedList, its capacity grows automatically.
+ * The complex number is stored using an non-interleaved format and so the maximum number of complex numbers that may be added is
+ * approximately 2<sup>31</sup>. This is also 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 ComplexNonInterleavedList instance before adding a large number of complex numbers
+ * using the ensureCapacityInternal operation. This may reduce the amount of incremental reallocation.</p>
+ *
+ * <p>This list does not support {@code null} Complex objects.
+ */
+ private static class ComplexNonInterleavedList extends ComplexList {
+
+ /** Max capacity for size of complex numbers in the list. */
+ private static final int MAX_CAPACITY = MAX_ARRAY_SIZE;
+ /** Error in case of allocation above max capacity. */
+ private static final String OOM_ERROR = OOM_ERROR_STRING + MAX_CAPACITY;
+ /**
+ * Storage for the real parts of complex numbers.
+ */
+ private double[] realParts;
+
Review Comment:
Delete this new line.
##########
commons-numbers-complex-arrays/src/test/java/org/apache/commons/numbers/complex/arrays/ComplexListTest.java:
##########
@@ -137,92 +256,119 @@ void testSetAddAndAddAllNullPointerException() {
list2.set(1, null);
Assertions.assertThrows(NullPointerException.class, () -> list1.addAll(list2));
Assertions.assertThrows(NullPointerException.class, () -> list1.addAll(0, list2));
+
+ // Check no modifications were made
Assertions.assertEquals(copy, list1);
}
- @Test
- void testRemove() {
- assertListOperation(list -> {
- list.add(Complex.ofCartesian(42, 13));
- list.addAll(list);
- list.remove(0);
- return list.remove(0);
- });
+ @ParameterizedTest
+ @MethodSource({"listImplementations"})
+ void testRemove(List<Complex> l2) {
+ List<Complex> l1 = new ArrayList<>();
+ assertListOperation(list -> list.add(Complex.ofCartesian(42, 13)), l1, l2);
+ assertListOperation(list -> list.addAll(list), l1, l2);
+ assertListOperation(list -> list.remove(0), l1, l2);
+ assertListOperation(list -> list.remove(0), l1, l2);
}
- @Test
- void testGetAndSetIndexOutOfBoundExceptions() {
- ComplexList list = ComplexList.interleaved();
+ @ParameterizedTest
+ @MethodSource({"listImplementations"})
+ void testGetAndSetIndexOutOfBoundExceptions(List<Complex> l) {
// Empty list throws
- Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list.get(0));
+ Assertions.assertThrows(IndexOutOfBoundsException.class, () -> l.get(0));
int size = 5;
- IntStream.range(0, size).mapToObj(i -> Complex.ofCartesian(i, -i)).forEach(list::add);
+ IntStream.range(0, size).mapToObj(i -> Complex.ofCartesian(i, -i)).forEach(l::add);
- Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list.get(-1));
- Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list.get(size));
- Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list.get(size + 1));
+ Assertions.assertThrows(IndexOutOfBoundsException.class, () -> l.get(-1));
+ Assertions.assertThrows(IndexOutOfBoundsException.class, () -> l.get(size));
+ Assertions.assertThrows(IndexOutOfBoundsException.class, () -> l.get(size + 1));
- Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list.set(-2, Complex.ofCartesian(200, 1)));
- Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list.set(size, Complex.ofCartesian(200, 1)));
- Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list.set(size + 1, Complex.ofCartesian(200, 1)));
+ Assertions.assertThrows(IndexOutOfBoundsException.class, () -> l.set(-2, Complex.ofCartesian(200, 1)));
+ Assertions.assertThrows(IndexOutOfBoundsException.class, () -> l.set(size, Complex.ofCartesian(200, 1)));
+ Assertions.assertThrows(IndexOutOfBoundsException.class, () -> l.set(size + 1, Complex.ofCartesian(200, 1)));
}
- @Test
- void testAddIndexOutOfBoundExceptions() {
- ComplexList list = ComplexList.interleaved();
+ @ParameterizedTest
+ @MethodSource({"listImplementations"})
+ void testAddIndexOutOfBoundExceptions(List<Complex> l) {
int size = 5;
- IntStream.range(0, size).mapToObj(i -> Complex.ofCartesian(i, -i)).forEach(list::add);
+ IntStream.range(0, size).mapToObj(i -> Complex.ofCartesian(i, -i)).forEach(l::add);
Assertions.assertThrows(IndexOutOfBoundsException.class, () ->
- list.add(-1, Complex.ofCartesian(42, 13)));
+ l.add(-1, Complex.ofCartesian(42, 13)));
Assertions.assertThrows(IndexOutOfBoundsException.class, () ->
- list.add(size + 1, Complex.ofCartesian(42, 13)));
+ l.add(size + 1, Complex.ofCartesian(42, 13)));
}
- @Test
- void testRemoveIndexOutOfBoundExceptions() {
- ComplexList list = generateComplexList(2);
- list.remove(0);
- Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list.remove(1));
- Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list.remove(-1));
+ @ParameterizedTest
+ @MethodSource({"listImplementationsWithSize"})
+ void testRemoveIndexOutOfBoundExceptions(List<Complex> l, int size) {
+ IntStream.range(0, size).forEach(i -> l.add(Complex.ofCartesian(i, -i)));
+ l.remove(0);
+ Assertions.assertThrows(IndexOutOfBoundsException.class, () -> l.remove(1));
+ Assertions.assertThrows(IndexOutOfBoundsException.class, () -> l.remove(-1));
}
@ParameterizedTest
@ValueSource(ints = {0, 1, 10})
void testConstructor(int size) {
List<Complex> l1 = new ArrayList<>(size);
List<Complex> l2 = ComplexList.interleaved(size);
+ List<Complex> l3 = new ArrayList<>(size);
+ List<Complex> l4 = ComplexList.nonInterleaved(size);
+
Assertions.assertEquals(l1, l2);
+ Assertions.assertEquals(l3, l4);
+
assertListOperation(l -> l.add(Complex.ofCartesian(10, 20)), l1, l2);
assertListOperation(l -> {
l.add(1, Complex.ofCartesian(10, 20));
return Boolean.TRUE;
}, l1, l2);
assertListOperation(l -> l.addAll(1, l), l1, l2);
+
+ assertListOperation(l -> l.add(Complex.ofCartesian(10, 20)), l3, l4);
+ assertListOperation(l -> {
+ l.add(1, Complex.ofCartesian(10, 20));
+ return Boolean.TRUE;
+ }, l3, l4);
+ assertListOperation(l -> l.addAll(1, l), l3, l4);
}
@Test
void testCapacityExceptions() {
- Assertions.assertThrows(IllegalArgumentException.class, () -> ComplexList.interleaved(MAX_CAPACITY + 1));
+ Assertions.assertThrows(IllegalArgumentException.class, () -> ComplexList.interleaved(
+ MAX_CAPACITY_INTERLEAVED + 1));
+ Assertions.assertThrows(IllegalArgumentException.class, () -> ComplexList.nonInterleaved(
+ MAX_CAPACITY_NON_INTERLEAVED + 1));
// Set-up required sizes
- ComplexList list = ComplexList.interleaved();
- List<Complex> l = new SizedList(Integer.MAX_VALUE);
- Assertions.assertThrows(OutOfMemoryError.class, () -> list.addAll(l));
-
- List<Complex> l2 = new SizedList(MAX_CAPACITY + 1);
- Assertions.assertThrows(OutOfMemoryError.class, () -> list.addAll(l2));
+ ComplexList list1 = ComplexList.interleaved();
+ ComplexList list2 = ComplexList.nonInterleaved();
+ List<Complex> l1 = new SizedList(Integer.MAX_VALUE);
+ Assertions.assertThrows(OutOfMemoryError.class, () -> list1.addAll(l1));
+ Assertions.assertThrows(OutOfMemoryError.class, () -> list2.addAll(l1));
+
+ List<Complex> l2 = new SizedList(MAX_CAPACITY_INTERLEAVED + 1);
+ List<Complex> l3 = new SizedList(MAX_CAPACITY_NON_INTERLEAVED + 1);
+ Assertions.assertThrows(OutOfMemoryError.class, () -> list1.addAll(l2));
+ Assertions.assertThrows(OutOfMemoryError.class, () -> list2.addAll(l3));
}
@Test
void testReplaceAllComplexUnaryOperator() {
Review Comment:
This should be parameterized
##########
commons-numbers-complex-arrays/src/test/java/org/apache/commons/numbers/complex/arrays/ComplexListTest.java:
##########
@@ -24,18 +24,79 @@
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
import org.junit.jupiter.params.provider.ValueSource;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.List;
import java.util.concurrent.ThreadLocalRandom;
import java.util.function.Function;
import java.util.stream.Collectors;
import java.util.stream.IntStream;
+import java.util.stream.Stream;
public class ComplexListTest {
- private static final int MAX_CAPACITY = (Integer.MAX_VALUE - 9) / 2;
+ private static final int MAX_CAPACITY_INTERLEAVED = (Integer.MAX_VALUE - 9) / 2;
+ private static final int MAX_CAPACITY_NON_INTERLEAVED = Integer.MAX_VALUE - 9;
+
+ /**
+ * Generate a stream of arguments containing empty {@code Complex<List>} implementations.
+ *
+ * @return the stream of arguments
+ */
+ static Stream<Arguments> listImplementations() {
+ return Stream.of(Arguments.of(ComplexList.interleaved()),
+ Arguments.of(ComplexList.nonInterleaved()));
+ }
+
+ /**
+ * Generate a stream of arguments containing {@code Complex<List>} implementations with a set size.
+ *
+ * @return the stream of arguments
+ */
+ static Stream<Arguments> listImplementationsWithSize() {
+ return Stream.of(Arguments.of(ComplexList.interleaved(), 2),
+ Arguments.of(ComplexList.nonInterleaved(), 2));
+ }
+
+ /**
+ * Generates a ComplexList in interleaved format of random complex numbers of the given size using the.
+ * @param size number of complex numbers in the list.
+ * @return the ComplexList of random complex numbers.
+ */
+ private static ComplexList generateComplexInterleavedList(int size) {
+ List<Complex> objectList = generateList(size);
+ ComplexList list = ComplexList.interleaved();
+ list.addAll(objectList);
+ Assertions.assertEquals(objectList, list);
+ return list;
+ }
+
+ /**
+ * Generates a ComplexList in non-interleaved format of random complex numbers of the given size.
+ * @param size number of complex numbers in the list.
+ * @return the ComplexList of random complex numbers.
+ */
+ private static ComplexList generateComplexNonInterleavedList(int size) {
+ List<Complex> objectList = generateList(size);
+ ComplexList list = ComplexList.nonInterleaved();
+ list.addAll(objectList);
+ Assertions.assertEquals(objectList, list);
+ return list;
+ }
+
+ /**
+ * Generates a list of random complex numbers of the given size.
+ * @param size number of complex numbers in the list.
+ * @return the list of random complex numbers.
+ */
+ private static List<Complex> generateList(int size) {
+ return ThreadLocalRandom.current().doubles(size, -Math.PI, Math.PI)
+ .mapToObj(Complex::ofCis).collect(Collectors.toList());
+ }
@Test
void testFromArray() {
Review Comment:
testFromArrayInterleaved
##########
commons-numbers-complex-arrays/src/test/java/org/apache/commons/numbers/complex/arrays/ComplexListTest.java:
##########
@@ -170,11 +233,21 @@ void testAddingEmptyListToEmptyList(List<Complex> l6) {
assertListOperation(l -> l.addAll(0, list), l5, l6);
}
- @Test
- void testSetAddAndAddAllNullPointerException() {
- ComplexList copy1 = ComplexList.interleaved();
- ComplexList list1 = generateComplexInterleavedList(3);
- copy1.addAll(list1);
+ @ParameterizedTest
+ @MethodSource({"listImplementationsWithSize"})
+ void testSetAddAndAddAllNullPointerException(List<Complex> list1, int size) {
Review Comment:
For consistency I would rename list1 -> list. The reference lists can be `expected`.
##########
commons-numbers-complex-arrays/src/main/java/org/apache/commons/numbers/complex/arrays/ComplexList.java:
##########
@@ -566,4 +604,299 @@ public void forEach(ComplexConsumer action) {
}
}
}
+
+ /**
+ * Resizable-double arrays 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 arrays that are used internally to store the list.
+ *
+ * <p>Each ComplexNonInterleavedList instance has a capacity. The capacity is the size of the double arrays used to store the complex numbers
+ * in the list. As complex numbers are added to an ComplexNonInterleavedList, its capacity grows automatically.
+ * The complex number is stored using an non-interleaved format and so the maximum number of complex numbers that may be added is
+ * approximately 2<sup>31</sup>. This is also 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 ComplexNonInterleavedList instance before adding a large number of complex numbers
+ * using the ensureCapacityInternal operation. This may reduce the amount of incremental reallocation.</p>
+ *
+ * <p>This list does not support {@code null} Complex objects.
+ */
+ private static class ComplexNonInterleavedList extends ComplexList {
+
+ /** Max capacity for size of complex numbers in the list. */
+ private static final int MAX_CAPACITY = MAX_ARRAY_SIZE;
+ /** Error in case of allocation above max capacity. */
+ private static final String OOM_ERROR = OOM_ERROR_STRING + MAX_CAPACITY;
+ /**
Review Comment:
Add new line here. This separates the grouping of the static fields from the instance fields
##########
commons-numbers-complex-arrays/src/test/java/org/apache/commons/numbers/complex/arrays/ComplexListTest.java:
##########
@@ -137,92 +256,119 @@ void testSetAddAndAddAllNullPointerException() {
list2.set(1, null);
Assertions.assertThrows(NullPointerException.class, () -> list1.addAll(list2));
Assertions.assertThrows(NullPointerException.class, () -> list1.addAll(0, list2));
+
+ // Check no modifications were made
Assertions.assertEquals(copy, list1);
}
- @Test
- void testRemove() {
- assertListOperation(list -> {
- list.add(Complex.ofCartesian(42, 13));
- list.addAll(list);
- list.remove(0);
- return list.remove(0);
- });
+ @ParameterizedTest
+ @MethodSource({"listImplementations"})
+ void testRemove(List<Complex> l2) {
+ List<Complex> l1 = new ArrayList<>();
+ assertListOperation(list -> list.add(Complex.ofCartesian(42, 13)), l1, l2);
+ assertListOperation(list -> list.addAll(list), l1, l2);
+ assertListOperation(list -> list.remove(0), l1, l2);
+ assertListOperation(list -> list.remove(0), l1, l2);
}
- @Test
- void testGetAndSetIndexOutOfBoundExceptions() {
- ComplexList list = ComplexList.interleaved();
+ @ParameterizedTest
+ @MethodSource({"listImplementations"})
+ void testGetAndSetIndexOutOfBoundExceptions(List<Complex> l) {
// Empty list throws
- Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list.get(0));
+ Assertions.assertThrows(IndexOutOfBoundsException.class, () -> l.get(0));
int size = 5;
- IntStream.range(0, size).mapToObj(i -> Complex.ofCartesian(i, -i)).forEach(list::add);
+ IntStream.range(0, size).mapToObj(i -> Complex.ofCartesian(i, -i)).forEach(l::add);
- Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list.get(-1));
- Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list.get(size));
- Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list.get(size + 1));
+ Assertions.assertThrows(IndexOutOfBoundsException.class, () -> l.get(-1));
+ Assertions.assertThrows(IndexOutOfBoundsException.class, () -> l.get(size));
+ Assertions.assertThrows(IndexOutOfBoundsException.class, () -> l.get(size + 1));
- Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list.set(-2, Complex.ofCartesian(200, 1)));
- Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list.set(size, Complex.ofCartesian(200, 1)));
- Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list.set(size + 1, Complex.ofCartesian(200, 1)));
+ Assertions.assertThrows(IndexOutOfBoundsException.class, () -> l.set(-2, Complex.ofCartesian(200, 1)));
+ Assertions.assertThrows(IndexOutOfBoundsException.class, () -> l.set(size, Complex.ofCartesian(200, 1)));
+ Assertions.assertThrows(IndexOutOfBoundsException.class, () -> l.set(size + 1, Complex.ofCartesian(200, 1)));
}
- @Test
- void testAddIndexOutOfBoundExceptions() {
- ComplexList list = ComplexList.interleaved();
+ @ParameterizedTest
+ @MethodSource({"listImplementations"})
+ void testAddIndexOutOfBoundExceptions(List<Complex> l) {
int size = 5;
- IntStream.range(0, size).mapToObj(i -> Complex.ofCartesian(i, -i)).forEach(list::add);
+ IntStream.range(0, size).mapToObj(i -> Complex.ofCartesian(i, -i)).forEach(l::add);
Assertions.assertThrows(IndexOutOfBoundsException.class, () ->
- list.add(-1, Complex.ofCartesian(42, 13)));
+ l.add(-1, Complex.ofCartesian(42, 13)));
Assertions.assertThrows(IndexOutOfBoundsException.class, () ->
- list.add(size + 1, Complex.ofCartesian(42, 13)));
+ l.add(size + 1, Complex.ofCartesian(42, 13)));
}
- @Test
- void testRemoveIndexOutOfBoundExceptions() {
- ComplexList list = generateComplexList(2);
- list.remove(0);
- Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list.remove(1));
- Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list.remove(-1));
+ @ParameterizedTest
+ @MethodSource({"listImplementationsWithSize"})
+ void testRemoveIndexOutOfBoundExceptions(List<Complex> l, int size) {
+ IntStream.range(0, size).forEach(i -> l.add(Complex.ofCartesian(i, -i)));
+ l.remove(0);
+ Assertions.assertThrows(IndexOutOfBoundsException.class, () -> l.remove(1));
+ Assertions.assertThrows(IndexOutOfBoundsException.class, () -> l.remove(-1));
}
@ParameterizedTest
@ValueSource(ints = {0, 1, 10})
void testConstructor(int size) {
Review Comment:
Note you should be able to stream a function reference as the argument to the parameterized test: IntToObjFunction<List<Complex>>:
```Java
void testConstructor(IntFunction<List<Complex>> constructor, int size) {
List<Complex> list = constructor.apply(size);
```
This requires:
```Java
Arguments.of((IntFunction<List<Complex>>) ComplexList::interleaved, 0),
Arguments.of((IntFunction<List<Complex>>) ComplexList::interleaved, 1), ...
```
##########
commons-numbers-complex-arrays/src/test/java/org/apache/commons/numbers/complex/arrays/ComplexListTest.java:
##########
@@ -137,92 +256,119 @@ void testSetAddAndAddAllNullPointerException() {
list2.set(1, null);
Assertions.assertThrows(NullPointerException.class, () -> list1.addAll(list2));
Assertions.assertThrows(NullPointerException.class, () -> list1.addAll(0, list2));
+
+ // Check no modifications were made
Assertions.assertEquals(copy, list1);
}
- @Test
- void testRemove() {
- assertListOperation(list -> {
- list.add(Complex.ofCartesian(42, 13));
- list.addAll(list);
- list.remove(0);
- return list.remove(0);
- });
+ @ParameterizedTest
+ @MethodSource({"listImplementations"})
+ void testRemove(List<Complex> l2) {
+ List<Complex> l1 = new ArrayList<>();
+ assertListOperation(list -> list.add(Complex.ofCartesian(42, 13)), l1, l2);
+ assertListOperation(list -> list.addAll(list), l1, l2);
+ assertListOperation(list -> list.remove(0), l1, l2);
+ assertListOperation(list -> list.remove(0), l1, l2);
}
- @Test
- void testGetAndSetIndexOutOfBoundExceptions() {
- ComplexList list = ComplexList.interleaved();
+ @ParameterizedTest
+ @MethodSource({"listImplementations"})
+ void testGetAndSetIndexOutOfBoundExceptions(List<Complex> l) {
// Empty list throws
- Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list.get(0));
+ Assertions.assertThrows(IndexOutOfBoundsException.class, () -> l.get(0));
int size = 5;
- IntStream.range(0, size).mapToObj(i -> Complex.ofCartesian(i, -i)).forEach(list::add);
+ IntStream.range(0, size).mapToObj(i -> Complex.ofCartesian(i, -i)).forEach(l::add);
- Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list.get(-1));
- Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list.get(size));
- Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list.get(size + 1));
+ Assertions.assertThrows(IndexOutOfBoundsException.class, () -> l.get(-1));
+ Assertions.assertThrows(IndexOutOfBoundsException.class, () -> l.get(size));
+ Assertions.assertThrows(IndexOutOfBoundsException.class, () -> l.get(size + 1));
- Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list.set(-2, Complex.ofCartesian(200, 1)));
- Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list.set(size, Complex.ofCartesian(200, 1)));
- Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list.set(size + 1, Complex.ofCartesian(200, 1)));
+ Assertions.assertThrows(IndexOutOfBoundsException.class, () -> l.set(-2, Complex.ofCartesian(200, 1)));
+ Assertions.assertThrows(IndexOutOfBoundsException.class, () -> l.set(size, Complex.ofCartesian(200, 1)));
+ Assertions.assertThrows(IndexOutOfBoundsException.class, () -> l.set(size + 1, Complex.ofCartesian(200, 1)));
}
- @Test
- void testAddIndexOutOfBoundExceptions() {
- ComplexList list = ComplexList.interleaved();
+ @ParameterizedTest
+ @MethodSource({"listImplementations"})
+ void testAddIndexOutOfBoundExceptions(List<Complex> l) {
int size = 5;
- IntStream.range(0, size).mapToObj(i -> Complex.ofCartesian(i, -i)).forEach(list::add);
+ IntStream.range(0, size).mapToObj(i -> Complex.ofCartesian(i, -i)).forEach(l::add);
Assertions.assertThrows(IndexOutOfBoundsException.class, () ->
- list.add(-1, Complex.ofCartesian(42, 13)));
+ l.add(-1, Complex.ofCartesian(42, 13)));
Assertions.assertThrows(IndexOutOfBoundsException.class, () ->
- list.add(size + 1, Complex.ofCartesian(42, 13)));
+ l.add(size + 1, Complex.ofCartesian(42, 13)));
}
- @Test
- void testRemoveIndexOutOfBoundExceptions() {
- ComplexList list = generateComplexList(2);
- list.remove(0);
- Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list.remove(1));
- Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list.remove(-1));
+ @ParameterizedTest
+ @MethodSource({"listImplementationsWithSize"})
+ void testRemoveIndexOutOfBoundExceptions(List<Complex> l, int size) {
+ IntStream.range(0, size).forEach(i -> l.add(Complex.ofCartesian(i, -i)));
+ l.remove(0);
+ Assertions.assertThrows(IndexOutOfBoundsException.class, () -> l.remove(1));
+ Assertions.assertThrows(IndexOutOfBoundsException.class, () -> l.remove(-1));
}
@ParameterizedTest
@ValueSource(ints = {0, 1, 10})
void testConstructor(int size) {
List<Complex> l1 = new ArrayList<>(size);
List<Complex> l2 = ComplexList.interleaved(size);
+ List<Complex> l3 = new ArrayList<>(size);
+ List<Complex> l4 = ComplexList.nonInterleaved(size);
+
Assertions.assertEquals(l1, l2);
+ Assertions.assertEquals(l3, l4);
+
assertListOperation(l -> l.add(Complex.ofCartesian(10, 20)), l1, l2);
assertListOperation(l -> {
l.add(1, Complex.ofCartesian(10, 20));
return Boolean.TRUE;
}, l1, l2);
assertListOperation(l -> l.addAll(1, l), l1, l2);
+
+ assertListOperation(l -> l.add(Complex.ofCartesian(10, 20)), l3, l4);
+ assertListOperation(l -> {
+ l.add(1, Complex.ofCartesian(10, 20));
+ return Boolean.TRUE;
+ }, l3, l4);
+ assertListOperation(l -> l.addAll(1, l), l3, l4);
}
@Test
void testCapacityExceptions() {
Review Comment:
Again this could be parameterized with the list constructor and an int containing the max capacity.
Note if you use the `@MethodSource` annotation without any arguments then the static method is assumed to be the same name as the test method. So you can have:
```Java
static Stream<Arguments> testCapacityExceptions() {
return Stream.of(
Arguments.of((IntFunction<List<Complex>>) ComplexList::interleaved, MAX_CAPACITY_INTERLEAVED),
Arguments.of((IntFunction<List<Complex>>) ComplexList::nonInterleaved, MAX_CAPACITY_NON_INTERLEAVED)
);
}
@ParameterizedTest
@MethodSource
void testCapacityExceptions(IntFunction<List<Complex>> constructor, int maxCapacity) {
Assertions.assertThrows(IllegalArgumentException.class, () -> constructor.apply(maxCapacity + 1));
```
##########
commons-numbers-complex-arrays/src/test/java/org/apache/commons/numbers/complex/arrays/ComplexListTest.java:
##########
@@ -231,114 +377,143 @@ void testReplaceAllComplexBinaryOperator() {
double r = 2;
double i = 3;
Complex multiplier = Complex.ofCartesian(r, i);
- ComplexList actualList = ComplexList.interleaved();
- actualList.addAll(objectList);
+ ComplexList actualList1 = ComplexList.interleaved();
Review Comment:
This should be parameterized as it just requires an empty list
##########
commons-numbers-complex-arrays/src/test/java/org/apache/commons/numbers/complex/arrays/ComplexListTest.java:
##########
@@ -101,34 +204,50 @@ void testAddAndAddAll() {
list.add(1, Complex.ofCartesian(10, 20));
return Boolean.TRUE;
}, l3, l4);
+ }
+ @ParameterizedTest
+ @MethodSource({"listImplementations"})
+ void testAddAndAddAllEnsureCapacityBranchConditions(List<Complex> l6) {
//Testing branch condition (newArrayCapacity < minArrayCapacity) in ensureCapacity
- ComplexList list1 = ComplexList.interleaved();
int size = 5;
+ List<Complex> list1 = new ArrayList<>(size);
IntStream.range(0, size).mapToObj(i -> Complex.ofCartesian(i, -i)).forEach(list1::add);
List<Complex> l5 = new ArrayList<>();
- List<Complex> l6 = ComplexList.interleaved();
assertListOperation(list -> list.add(Complex.ofCartesian(1, 2)), l5, l6);
// Expand the list by doubling in size until at the known minArrayCapacity
while (l5.size() < 8) {
assertListOperation(list -> list.addAll(list), l5, l6);
}
assertListOperation(list -> list.addAll(list1), l5, l6);
+ }
+ @ParameterizedTest
+ @MethodSource({"listImplementations"})
+ void testAddingEmptyListToEmptyList(List<Complex> l6) {
Review Comment:
Rename l6 -> l2; l5 -> l1
##########
commons-numbers-complex-arrays/src/test/java/org/apache/commons/numbers/complex/arrays/ComplexListTest.java:
##########
@@ -101,34 +204,50 @@ void testAddAndAddAll() {
list.add(1, Complex.ofCartesian(10, 20));
return Boolean.TRUE;
}, l3, l4);
+ }
+ @ParameterizedTest
+ @MethodSource({"listImplementations"})
+ void testAddAndAddAllEnsureCapacityBranchConditions(List<Complex> l6) {
Review Comment:
testAddAllEnsureCapacityBranchConditions
Rename l6 -> l2; l5 -> l1
##########
commons-numbers-complex-arrays/src/test/java/org/apache/commons/numbers/complex/arrays/ComplexListTest.java:
##########
@@ -79,10 +179,13 @@ void testAddAndAddAll() {
assertListOperation(list -> list.add(Complex.ofCartesian(19, 20)), l1, l2);
assertListOperation(list -> list.add(Complex.ofCartesian(21, 22)), l1, l2);
assertListOperation(list -> list.add(Complex.ofCartesian(23, 24)), l1, l2);
+ }
- //Testing add at an index for branch condition (size == realAndImagParts.length >>> 1)
+ @ParameterizedTest
+ @MethodSource({"listImplementations"})
+ void testAddAndAddAtIndexBranchCondition(List<Complex> l4) {
Review Comment:
Rename l4 -> l2; l3 -> l1
##########
commons-numbers-complex-arrays/src/test/java/org/apache/commons/numbers/complex/arrays/ComplexListTest.java:
##########
@@ -231,114 +377,143 @@ void testReplaceAllComplexBinaryOperator() {
double r = 2;
double i = 3;
Complex multiplier = Complex.ofCartesian(r, i);
- ComplexList actualList = ComplexList.interleaved();
- actualList.addAll(objectList);
+ ComplexList actualList1 = ComplexList.interleaved();
+ ComplexList actualList2 = ComplexList.nonInterleaved();
+ actualList1.addAll(objectList);
+ actualList2.addAll(objectList);
objectList.replaceAll(c -> c.multiply(multiplier));
- actualList.replaceAll((x, y, action) -> ComplexFunctions.multiply(x, y, r, i, action));
- Assertions.assertEquals(objectList, actualList);
+ actualList1.replaceAll((x, y, action) -> ComplexFunctions.multiply(x, y, r, i, action));
+ actualList2.replaceAll((x, y, action) -> ComplexFunctions.multiply(x, y, r, i, action));
+ Assertions.assertEquals(objectList, actualList1);
+ Assertions.assertEquals(objectList, actualList2);
}
@Test
void testReplaceAllComplexScalarFunction() {
List<Complex> objectList = generateList(10);
double factor = 2;
- ComplexList actualList = ComplexList.interleaved();
- actualList.addAll(objectList);
+ ComplexList actualList1 = ComplexList.interleaved();
+ ComplexList actualList2 = ComplexList.nonInterleaved();
+ actualList1.addAll(objectList);
+ actualList2.addAll(objectList);
objectList.replaceAll(c -> c.pow(factor));
- actualList.replaceAll((x, y, action) -> ComplexFunctions.pow(x, y, factor, action));
- Assertions.assertEquals(objectList, actualList);
+ actualList1.replaceAll((x, y, action) -> ComplexFunctions.pow(x, y, factor, action));
+ actualList2.replaceAll((x, y, action) -> ComplexFunctions.pow(x, y, factor, action));
+ Assertions.assertEquals(objectList, actualList1);
+ Assertions.assertEquals(objectList, actualList2);
}
@ParameterizedTest
@ValueSource(ints = {0, 10})
void testForEachComplexConsumer(int size) {
Review Comment:
This should be parameterized with the empty list and a size argument
##########
commons-numbers-complex-arrays/src/test/java/org/apache/commons/numbers/complex/arrays/ComplexListTest.java:
##########
@@ -231,114 +377,143 @@ void testReplaceAllComplexBinaryOperator() {
double r = 2;
double i = 3;
Complex multiplier = Complex.ofCartesian(r, i);
- ComplexList actualList = ComplexList.interleaved();
- actualList.addAll(objectList);
+ ComplexList actualList1 = ComplexList.interleaved();
+ ComplexList actualList2 = ComplexList.nonInterleaved();
+ actualList1.addAll(objectList);
+ actualList2.addAll(objectList);
objectList.replaceAll(c -> c.multiply(multiplier));
- actualList.replaceAll((x, y, action) -> ComplexFunctions.multiply(x, y, r, i, action));
- Assertions.assertEquals(objectList, actualList);
+ actualList1.replaceAll((x, y, action) -> ComplexFunctions.multiply(x, y, r, i, action));
+ actualList2.replaceAll((x, y, action) -> ComplexFunctions.multiply(x, y, r, i, action));
+ Assertions.assertEquals(objectList, actualList1);
+ Assertions.assertEquals(objectList, actualList2);
}
@Test
void testReplaceAllComplexScalarFunction() {
List<Complex> objectList = generateList(10);
double factor = 2;
- ComplexList actualList = ComplexList.interleaved();
- actualList.addAll(objectList);
+ ComplexList actualList1 = ComplexList.interleaved();
+ ComplexList actualList2 = ComplexList.nonInterleaved();
+ actualList1.addAll(objectList);
+ actualList2.addAll(objectList);
objectList.replaceAll(c -> c.pow(factor));
- actualList.replaceAll((x, y, action) -> ComplexFunctions.pow(x, y, factor, action));
- Assertions.assertEquals(objectList, actualList);
+ actualList1.replaceAll((x, y, action) -> ComplexFunctions.pow(x, y, factor, action));
+ actualList2.replaceAll((x, y, action) -> ComplexFunctions.pow(x, y, factor, action));
+ Assertions.assertEquals(objectList, actualList1);
+ Assertions.assertEquals(objectList, actualList2);
}
@ParameterizedTest
@ValueSource(ints = {0, 10})
void testForEachComplexConsumer(int size) {
- ComplexList expected = generateComplexList(size);
- ArrayList<Complex> actual = new ArrayList<>();
- Assertions.assertThrows(NullPointerException.class, () -> expected.forEach((ComplexConsumer) null));
- expected.forEach((real, imaginary) -> actual.add(Complex.ofCartesian(real, imaginary)));
- Assertions.assertEquals(expected, actual);
+ ComplexList expected1 = generateComplexInterleavedList(size);
+ ComplexList expected2 = generateComplexNonInterleavedList(size);
+ ArrayList<Complex> actual1 = new ArrayList<>();
+ ArrayList<Complex> actual2 = new ArrayList<>();
+ Assertions.assertThrows(NullPointerException.class, () -> expected1.forEach((ComplexConsumer) null));
+ Assertions.assertThrows(NullPointerException.class, () -> expected2.forEach((ComplexConsumer) null));
+ expected1.forEach((real, imaginary) -> actual1.add(Complex.ofCartesian(real, imaginary)));
+ expected2.forEach((real, imaginary) -> actual2.add(Complex.ofCartesian(real, imaginary)));
+ Assertions.assertEquals(expected1, actual1);
+ Assertions.assertEquals(expected2, actual2);
}
@Test
void testGetRealAndImaginary() {
- ComplexList list = generateComplexList(10);
- for (int i = 0; i < list.size(); i++) {
- Assertions.assertEquals(list.get(i).getReal(), list.getReal(i), "real");
- Assertions.assertEquals(list.get(i).getImaginary(), list.getImaginary(i), "imaginary");
+ int size = 10;
+ ComplexList list1 = generateComplexInterleavedList(size);
+ ComplexList list2 = generateComplexNonInterleavedList(size);
+ for (int i = 0; i < size; i++) {
+ Assertions.assertEquals(list1.get(i).getReal(), list1.getReal(i), "real");
+ Assertions.assertEquals(list1.get(i).getImaginary(), list1.getImaginary(i), "imaginary");
+ Assertions.assertEquals(list2.get(i).getReal(), list2.getReal(i), "real");
+ Assertions.assertEquals(list2.get(i).getImaginary(), list2.getImaginary(i), "imaginary");
}
}
@Test
void testSetRealAndImaginary() {
Review Comment:
This should be parameterized with an empty list and a size argument
##########
commons-numbers-complex-arrays/src/test/java/org/apache/commons/numbers/complex/arrays/ComplexListTest.java:
##########
@@ -231,114 +377,143 @@ void testReplaceAllComplexBinaryOperator() {
double r = 2;
double i = 3;
Complex multiplier = Complex.ofCartesian(r, i);
- ComplexList actualList = ComplexList.interleaved();
- actualList.addAll(objectList);
+ ComplexList actualList1 = ComplexList.interleaved();
+ ComplexList actualList2 = ComplexList.nonInterleaved();
+ actualList1.addAll(objectList);
+ actualList2.addAll(objectList);
objectList.replaceAll(c -> c.multiply(multiplier));
- actualList.replaceAll((x, y, action) -> ComplexFunctions.multiply(x, y, r, i, action));
- Assertions.assertEquals(objectList, actualList);
+ actualList1.replaceAll((x, y, action) -> ComplexFunctions.multiply(x, y, r, i, action));
+ actualList2.replaceAll((x, y, action) -> ComplexFunctions.multiply(x, y, r, i, action));
+ Assertions.assertEquals(objectList, actualList1);
+ Assertions.assertEquals(objectList, actualList2);
}
@Test
void testReplaceAllComplexScalarFunction() {
Review Comment:
This should be parameterized
##########
commons-numbers-complex-arrays/src/test/java/org/apache/commons/numbers/complex/arrays/ComplexListTest.java:
##########
@@ -231,114 +377,143 @@ void testReplaceAllComplexBinaryOperator() {
double r = 2;
double i = 3;
Complex multiplier = Complex.ofCartesian(r, i);
- ComplexList actualList = ComplexList.interleaved();
- actualList.addAll(objectList);
+ ComplexList actualList1 = ComplexList.interleaved();
+ ComplexList actualList2 = ComplexList.nonInterleaved();
+ actualList1.addAll(objectList);
+ actualList2.addAll(objectList);
objectList.replaceAll(c -> c.multiply(multiplier));
- actualList.replaceAll((x, y, action) -> ComplexFunctions.multiply(x, y, r, i, action));
- Assertions.assertEquals(objectList, actualList);
+ actualList1.replaceAll((x, y, action) -> ComplexFunctions.multiply(x, y, r, i, action));
+ actualList2.replaceAll((x, y, action) -> ComplexFunctions.multiply(x, y, r, i, action));
+ Assertions.assertEquals(objectList, actualList1);
+ Assertions.assertEquals(objectList, actualList2);
}
@Test
void testReplaceAllComplexScalarFunction() {
List<Complex> objectList = generateList(10);
double factor = 2;
- ComplexList actualList = ComplexList.interleaved();
- actualList.addAll(objectList);
+ ComplexList actualList1 = ComplexList.interleaved();
+ ComplexList actualList2 = ComplexList.nonInterleaved();
+ actualList1.addAll(objectList);
+ actualList2.addAll(objectList);
objectList.replaceAll(c -> c.pow(factor));
- actualList.replaceAll((x, y, action) -> ComplexFunctions.pow(x, y, factor, action));
- Assertions.assertEquals(objectList, actualList);
+ actualList1.replaceAll((x, y, action) -> ComplexFunctions.pow(x, y, factor, action));
+ actualList2.replaceAll((x, y, action) -> ComplexFunctions.pow(x, y, factor, action));
+ Assertions.assertEquals(objectList, actualList1);
+ Assertions.assertEquals(objectList, actualList2);
}
@ParameterizedTest
@ValueSource(ints = {0, 10})
void testForEachComplexConsumer(int size) {
- ComplexList expected = generateComplexList(size);
- ArrayList<Complex> actual = new ArrayList<>();
- Assertions.assertThrows(NullPointerException.class, () -> expected.forEach((ComplexConsumer) null));
- expected.forEach((real, imaginary) -> actual.add(Complex.ofCartesian(real, imaginary)));
- Assertions.assertEquals(expected, actual);
+ ComplexList expected1 = generateComplexInterleavedList(size);
+ ComplexList expected2 = generateComplexNonInterleavedList(size);
+ ArrayList<Complex> actual1 = new ArrayList<>();
+ ArrayList<Complex> actual2 = new ArrayList<>();
+ Assertions.assertThrows(NullPointerException.class, () -> expected1.forEach((ComplexConsumer) null));
+ Assertions.assertThrows(NullPointerException.class, () -> expected2.forEach((ComplexConsumer) null));
+ expected1.forEach((real, imaginary) -> actual1.add(Complex.ofCartesian(real, imaginary)));
+ expected2.forEach((real, imaginary) -> actual2.add(Complex.ofCartesian(real, imaginary)));
+ Assertions.assertEquals(expected1, actual1);
+ Assertions.assertEquals(expected2, actual2);
}
@Test
void testGetRealAndImaginary() {
- ComplexList list = generateComplexList(10);
- for (int i = 0; i < list.size(); i++) {
- Assertions.assertEquals(list.get(i).getReal(), list.getReal(i), "real");
- Assertions.assertEquals(list.get(i).getImaginary(), list.getImaginary(i), "imaginary");
+ int size = 10;
+ ComplexList list1 = generateComplexInterleavedList(size);
+ ComplexList list2 = generateComplexNonInterleavedList(size);
+ for (int i = 0; i < size; i++) {
+ Assertions.assertEquals(list1.get(i).getReal(), list1.getReal(i), "real");
+ Assertions.assertEquals(list1.get(i).getImaginary(), list1.getImaginary(i), "imaginary");
+ Assertions.assertEquals(list2.get(i).getReal(), list2.getReal(i), "real");
+ Assertions.assertEquals(list2.get(i).getImaginary(), list2.getImaginary(i), "imaginary");
}
}
@Test
void testSetRealAndImaginary() {
- ComplexList list = generateComplexList(10);
- for (int i = 0; i < list.size(); i++) {
+ int size = 10;
+ ComplexList list1 = generateComplexInterleavedList(size);
+ ComplexList list2 = generateComplexNonInterleavedList(size);
+ for (int i = 0; i < size; i++) {
final double value = Math.PI * i;
- list.setReal(i, value);
- list.setImaginary(i, value);
- Assertions.assertEquals(value, list.get(i).getReal());
- Assertions.assertEquals(value, list.get(i).getImaginary());
+ list1.setReal(i, value);
+ list1.setImaginary(i, value);
+ list2.setReal(i, value);
+ list2.setImaginary(i, value);
+ Assertions.assertEquals(value, list1.get(i).getReal());
+ Assertions.assertEquals(value, list1.get(i).getImaginary());
+ Assertions.assertEquals(value, list2.get(i).getReal());
+ Assertions.assertEquals(value, list2.get(i).getImaginary());
}
}
@Test
void testGetAndSetRealAndImaginaryIndexOutOfBoundsException() {
Review Comment:
This should be parameterized
##########
commons-numbers-complex-arrays/src/test/java/org/apache/commons/numbers/complex/arrays/ComplexListTest.java:
##########
@@ -231,114 +377,143 @@ void testReplaceAllComplexBinaryOperator() {
double r = 2;
double i = 3;
Complex multiplier = Complex.ofCartesian(r, i);
- ComplexList actualList = ComplexList.interleaved();
- actualList.addAll(objectList);
+ ComplexList actualList1 = ComplexList.interleaved();
+ ComplexList actualList2 = ComplexList.nonInterleaved();
+ actualList1.addAll(objectList);
+ actualList2.addAll(objectList);
objectList.replaceAll(c -> c.multiply(multiplier));
- actualList.replaceAll((x, y, action) -> ComplexFunctions.multiply(x, y, r, i, action));
- Assertions.assertEquals(objectList, actualList);
+ actualList1.replaceAll((x, y, action) -> ComplexFunctions.multiply(x, y, r, i, action));
+ actualList2.replaceAll((x, y, action) -> ComplexFunctions.multiply(x, y, r, i, action));
+ Assertions.assertEquals(objectList, actualList1);
+ Assertions.assertEquals(objectList, actualList2);
}
@Test
void testReplaceAllComplexScalarFunction() {
List<Complex> objectList = generateList(10);
double factor = 2;
- ComplexList actualList = ComplexList.interleaved();
- actualList.addAll(objectList);
+ ComplexList actualList1 = ComplexList.interleaved();
+ ComplexList actualList2 = ComplexList.nonInterleaved();
+ actualList1.addAll(objectList);
+ actualList2.addAll(objectList);
objectList.replaceAll(c -> c.pow(factor));
- actualList.replaceAll((x, y, action) -> ComplexFunctions.pow(x, y, factor, action));
- Assertions.assertEquals(objectList, actualList);
+ actualList1.replaceAll((x, y, action) -> ComplexFunctions.pow(x, y, factor, action));
+ actualList2.replaceAll((x, y, action) -> ComplexFunctions.pow(x, y, factor, action));
+ Assertions.assertEquals(objectList, actualList1);
+ Assertions.assertEquals(objectList, actualList2);
}
@ParameterizedTest
@ValueSource(ints = {0, 10})
void testForEachComplexConsumer(int size) {
- ComplexList expected = generateComplexList(size);
- ArrayList<Complex> actual = new ArrayList<>();
- Assertions.assertThrows(NullPointerException.class, () -> expected.forEach((ComplexConsumer) null));
- expected.forEach((real, imaginary) -> actual.add(Complex.ofCartesian(real, imaginary)));
- Assertions.assertEquals(expected, actual);
+ ComplexList expected1 = generateComplexInterleavedList(size);
+ ComplexList expected2 = generateComplexNonInterleavedList(size);
+ ArrayList<Complex> actual1 = new ArrayList<>();
+ ArrayList<Complex> actual2 = new ArrayList<>();
+ Assertions.assertThrows(NullPointerException.class, () -> expected1.forEach((ComplexConsumer) null));
+ Assertions.assertThrows(NullPointerException.class, () -> expected2.forEach((ComplexConsumer) null));
+ expected1.forEach((real, imaginary) -> actual1.add(Complex.ofCartesian(real, imaginary)));
+ expected2.forEach((real, imaginary) -> actual2.add(Complex.ofCartesian(real, imaginary)));
+ Assertions.assertEquals(expected1, actual1);
+ Assertions.assertEquals(expected2, actual2);
}
@Test
void testGetRealAndImaginary() {
- ComplexList list = generateComplexList(10);
- for (int i = 0; i < list.size(); i++) {
- Assertions.assertEquals(list.get(i).getReal(), list.getReal(i), "real");
- Assertions.assertEquals(list.get(i).getImaginary(), list.getImaginary(i), "imaginary");
+ int size = 10;
+ ComplexList list1 = generateComplexInterleavedList(size);
+ ComplexList list2 = generateComplexNonInterleavedList(size);
+ for (int i = 0; i < size; i++) {
+ Assertions.assertEquals(list1.get(i).getReal(), list1.getReal(i), "real");
+ Assertions.assertEquals(list1.get(i).getImaginary(), list1.getImaginary(i), "imaginary");
+ Assertions.assertEquals(list2.get(i).getReal(), list2.getReal(i), "real");
+ Assertions.assertEquals(list2.get(i).getImaginary(), list2.getImaginary(i), "imaginary");
}
}
@Test
void testSetRealAndImaginary() {
- ComplexList list = generateComplexList(10);
- for (int i = 0; i < list.size(); i++) {
+ int size = 10;
+ ComplexList list1 = generateComplexInterleavedList(size);
+ ComplexList list2 = generateComplexNonInterleavedList(size);
+ for (int i = 0; i < size; i++) {
final double value = Math.PI * i;
- list.setReal(i, value);
- list.setImaginary(i, value);
- Assertions.assertEquals(value, list.get(i).getReal());
- Assertions.assertEquals(value, list.get(i).getImaginary());
+ list1.setReal(i, value);
+ list1.setImaginary(i, value);
+ list2.setReal(i, value);
+ list2.setImaginary(i, value);
+ Assertions.assertEquals(value, list1.get(i).getReal());
+ Assertions.assertEquals(value, list1.get(i).getImaginary());
+ Assertions.assertEquals(value, list2.get(i).getReal());
+ Assertions.assertEquals(value, list2.get(i).getImaginary());
}
}
@Test
void testGetAndSetRealAndImaginaryIndexOutOfBoundsException() {
- ComplexList list = ComplexList.interleaved();
+ ComplexList list1 = ComplexList.interleaved();
+ ComplexList list2 = ComplexList.nonInterleaved();
// Empty list throws
- Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list.getReal(0));
- Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list.getImaginary(0));
+ Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list1.getReal(0));
+ Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list1.getImaginary(0));
+
+ Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list2.getReal(0));
+ Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list2.getImaginary(0));
int size = 5;
- IntStream.range(0, size).mapToObj(i -> Complex.ofCartesian(i, -i)).forEach(list::add);
+ IntStream.range(0, size).mapToObj(i -> Complex.ofCartesian(i, -i)).forEach(list1::add);
+ IntStream.range(0, size).mapToObj(i -> Complex.ofCartesian(i, -i)).forEach(list2::add);
+
+ Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list1.getReal(-1));
+ Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list1.getReal(size));
+ Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list1.getReal(size + 1));
+
+ Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list2.getReal(-1));
+ Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list2.getReal(size));
+ Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list2.getReal(size + 1));
+
+ Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list1.getImaginary(-1));
+ Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list1.getImaginary(size));
+ Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list1.getImaginary(size + 1));
- Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list.getReal(-1));
- Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list.getReal(size));
- Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list.getReal(size + 1));
+ Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list2.getImaginary(-1));
+ Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list2.getImaginary(size));
+ Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list2.getImaginary(size + 1));
- Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list.getImaginary(-1));
- Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list.getImaginary(size));
- Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list.getImaginary(size + 1));
+ Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list1.setReal(-2, 200));
+ Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list1.setReal(size, 200));
+ Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list1.setReal(size + 1, 200));
- Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list.setReal(-2, 200));
- Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list.setReal(size, 200));
- Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list.setReal(size + 1, 200));
+ Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list2.setReal(-2, 200));
+ Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list2.setReal(size, 200));
+ Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list2.setReal(size + 1, 200));
- Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list.setImaginary(-2, 200));
- Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list.setImaginary(size, 200));
- Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list.setImaginary(size + 1, 200));
+ Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list1.setImaginary(-2, 200));
+ Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list1.setImaginary(size, 200));
+ Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list1.setImaginary(size + 1, 200));
+
+ Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list2.setImaginary(-2, 200));
+ Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list2.setImaginary(size, 200));
+ Assertions.assertThrows(IndexOutOfBoundsException.class, () -> list2.setImaginary(size + 1, 200));
}
@Test
void testToArrayRealAndImaginary() {
Review Comment:
This should be parameterized
##########
commons-numbers-complex-arrays/src/test/java/org/apache/commons/numbers/complex/arrays/ComplexListTest.java:
##########
@@ -231,114 +377,143 @@ void testReplaceAllComplexBinaryOperator() {
double r = 2;
double i = 3;
Complex multiplier = Complex.ofCartesian(r, i);
- ComplexList actualList = ComplexList.interleaved();
- actualList.addAll(objectList);
+ ComplexList actualList1 = ComplexList.interleaved();
+ ComplexList actualList2 = ComplexList.nonInterleaved();
+ actualList1.addAll(objectList);
+ actualList2.addAll(objectList);
objectList.replaceAll(c -> c.multiply(multiplier));
- actualList.replaceAll((x, y, action) -> ComplexFunctions.multiply(x, y, r, i, action));
- Assertions.assertEquals(objectList, actualList);
+ actualList1.replaceAll((x, y, action) -> ComplexFunctions.multiply(x, y, r, i, action));
+ actualList2.replaceAll((x, y, action) -> ComplexFunctions.multiply(x, y, r, i, action));
+ Assertions.assertEquals(objectList, actualList1);
+ Assertions.assertEquals(objectList, actualList2);
}
@Test
void testReplaceAllComplexScalarFunction() {
List<Complex> objectList = generateList(10);
double factor = 2;
- ComplexList actualList = ComplexList.interleaved();
- actualList.addAll(objectList);
+ ComplexList actualList1 = ComplexList.interleaved();
+ ComplexList actualList2 = ComplexList.nonInterleaved();
+ actualList1.addAll(objectList);
+ actualList2.addAll(objectList);
objectList.replaceAll(c -> c.pow(factor));
- actualList.replaceAll((x, y, action) -> ComplexFunctions.pow(x, y, factor, action));
- Assertions.assertEquals(objectList, actualList);
+ actualList1.replaceAll((x, y, action) -> ComplexFunctions.pow(x, y, factor, action));
+ actualList2.replaceAll((x, y, action) -> ComplexFunctions.pow(x, y, factor, action));
+ Assertions.assertEquals(objectList, actualList1);
+ Assertions.assertEquals(objectList, actualList2);
}
@ParameterizedTest
@ValueSource(ints = {0, 10})
void testForEachComplexConsumer(int size) {
- ComplexList expected = generateComplexList(size);
- ArrayList<Complex> actual = new ArrayList<>();
- Assertions.assertThrows(NullPointerException.class, () -> expected.forEach((ComplexConsumer) null));
- expected.forEach((real, imaginary) -> actual.add(Complex.ofCartesian(real, imaginary)));
- Assertions.assertEquals(expected, actual);
+ ComplexList expected1 = generateComplexInterleavedList(size);
+ ComplexList expected2 = generateComplexNonInterleavedList(size);
+ ArrayList<Complex> actual1 = new ArrayList<>();
+ ArrayList<Complex> actual2 = new ArrayList<>();
+ Assertions.assertThrows(NullPointerException.class, () -> expected1.forEach((ComplexConsumer) null));
+ Assertions.assertThrows(NullPointerException.class, () -> expected2.forEach((ComplexConsumer) null));
+ expected1.forEach((real, imaginary) -> actual1.add(Complex.ofCartesian(real, imaginary)));
+ expected2.forEach((real, imaginary) -> actual2.add(Complex.ofCartesian(real, imaginary)));
+ Assertions.assertEquals(expected1, actual1);
+ Assertions.assertEquals(expected2, actual2);
}
@Test
void testGetRealAndImaginary() {
Review Comment:
This should be parameterized
--
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