Thanks Alex for PR feedback.
I have incorporated the majority of the requested changes to the PR.
I would like to discuss the remaining points here.
> In
commons-numbers-complex/src/main/java/org/apache/commons/numbers/complex/ComplexUnaryOperator.java:
> Note that by making this a default that creates an instance of Complex
you impose memory allocation overhead to any call site that just has the
real and imaginary parts (e.g. a structure storing a list of complex
numbers using primitive arrays).
> The ComplexDouble apply(ComplexDouble in,
ComplexConstructor<ComplexDouble> out) method should be a default that
calls this function using the real and imaginary parts.
The existing unit test for Complex Projection (proj) is expecting the
returned result to be the same as the passed input Complex instance except
for the isInfinite edge case.
This was the main reason for making the functional interface accept a
ComplexDouble instead of primitive real and imaginary parts.
The additional memory allocation for the input can be eliminated (for the
example of primitive array backed lists), by combining it with the
constructed result object.
We had decided to incur the overhead for the complex Constructor
result object in the context of functional compositions and thread safety
discussion before.
So, passing in a cursor iterator item (that captures the index) as both
ComplexDouble input and ComplexResult constructor should avoid any
additional overhead?
If we decide to use primitive types for the functional interface, then we
cannot use it for the projection method.
> @FunctionalInterface
> public interface ComplexScalarFunction {
>
> ComplexDouble apply(ComplexDouble c, double f,
ComplexConstructor<ComplexDouble> result);
>
> }
> This interface should be typed: the result is accepted by the
ComplexConstructor and this can be typed.
By typed, did you mean to make this a generic interface
ComplexScalarFunction<T>
?
ComplexUnaryOperator and ComplexBinaryOperator are not generic and
constrained to ComplexDouble types
> It may be wise to update the test suite to ensure that all tests
currently applied to Complex are applied to ComplexFunctions using a
ComplexConstructor other than Complex, for example using a dummy
implementation:
>
> class ComplexNumber implements ComplexConstructor<ComplexNumber> {
> // (r, i) members
>
> @Override
> public ComplexNumber apply(double r, double i) {
> // store (r, i) ...
> return this;
> }
> }
>
>This will detect the edge cases all pass through to the input constructor
to create the result. The test should assert the ComplexConstructor
received the expected values.
For now, shall I copy all the tests from the CStandardTest, CReferenceTest
and ComplexEdgeCaseTest to ComplexFunctionsTest and modify it to use a
dummy constructor?
Also please note as mentioned above for the complex.proj method, currently
we are not invoking the complex constructor for normal cases.
