> On 7 Nov 2019, at 00:28, Gilles Sadowski <gillese...@gmail.com> wrote:
>
> 2019-11-06 23:38 UTC+01:00, Alex Herbert <alex.d.herb...@gmail.com
> <mailto:alex.d.herb...@gmail.com>>:
>>
>>
>>> On 6 Nov 2019, at 18:17, Gilles Sadowski <gillese...@gmail.com> wrote:
>>>
>>>> [...]
>>>>
>>>>
>>>> Any objections to updating multiply/divide/isNaN to match the standard?
>>>
>>> Let me think... ;-)
>>
>> OK, I’ll fix it and double check the other tests against the c reference.
>>
>>>
>>>>
>>>> I'll add unit tests to hit the edge cases that should fail with the
>>>> current implementation.
>>>
>>> Thanks,
>>> Gilles
>>
>> Are changes to numbers going under Jira tickets?
>
> There is a JIRA project:
> https://issues.apache.org/jira/projects/NUMBERS
> <https://issues.apache.org/jira/projects/NUMBERS>
> for reference in the git commit (for non-trivial changes).
>
> But we don't really keep track (no "changes.xml") until the first
> official release (getting close now, I hope...).
>
>>
>> It looks like it needs an update to checkstyle, PMD, spotbugs, the
>> commons-parent and travis.
>
> Yes. I try to copy from or refer to Commons RNG.
>
>> Checkstyle config from commons-rng finds:
>>
>> [INFO] There are 115 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-core/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>> [INFO] There are 202 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-complex/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>> [INFO] There are 102 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-complex-streams/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>> [INFO] There are 276 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-primes/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>> [INFO] There are 68 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-quaternion/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>> [INFO] There are 289 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-fraction/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>> [INFO] There are 10 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-angle/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>> [INFO] There are 3503 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-gamma/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>> [INFO] There are 56 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-combinatorics/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>> [INFO] There are 50 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-arrays/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>> [INFO] There are 10 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-field/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>> [INFO] There are 4 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-rootfinder/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>>
>> The mass of errors is white space style in the test classes.
>
> The bulk of the tests was ported from Commons Math where
> CheckStyle was not enforced for unit test classes.
>
>> Without the
>> test classes the result is:
>>
>> [INFO] There are 12 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-core/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>> [INFO] There are 54 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-complex/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>> [INFO] There are 19 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-complex-streams/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>> [INFO] There are 49 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-primes/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>> [INFO] There are 5 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-quaternion/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>> [INFO] There are 6 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-fraction/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>> [INFO] There are 3 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-angle/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>> [INFO] There are 20 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-gamma/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>> [INFO] There are 19 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-combinatorics/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>> [INFO] There are 4 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-arrays/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>> [INFO] There are 6 errors reported by Checkstyle 8.20 with
>> /Users/ah403/git/commons-numbers/commons-numbers-field/../src/main/resources/checkstyle/checkstyle.xml
>> ruleset.
>
> Yes, those should be fixed. It's on the TODO list (among other tasks):
> https://issues.apache.org/jira/projects/NUMBERS/issues/NUMBERS-25
> <https://issues.apache.org/jira/projects/NUMBERS/issues/NUMBERS-25>
>
>>
>> Also looking at Complex it would benefit from:
>>
>> public Complex subtractFrom(double minuend) {
>> return new Complex(minuend - real, imaginary);
>> }
>
> Is it part of the "standard"?
> IMHO, it's fairly confusing, as nothing in the name indicates
> that the operation only applies to the real part.
The same applies for the add(real) and subtract(real) methods.
What is confusing is the unconventional reverse ordering of z.subtractFrom(a)
== a - z. I’m not happy about it but the javadoc can explain the outcome.
It is possible in C:
#include <stdio.h>
#include <complex.h>
#include <math.h>
#include <float.h>
int main( int argc, const char* argv[] ) {
double a = 5.0;
complex double z = 1 + 3 * I;
complex double z2 = a - z;
printf("%.17g + %.17g i\n", creal(z2), cimag(z2));
complex double z3 = z - a;
printf("%.17g + %.17g i\n", creal(z3), cimag(z3));
}
And C++:
#include <complex>
#include <math.h>
#include <float.h>
int main( int argc, const char* argv[] ) {
const double a = 5.0;
std::complex<double> z(1, 3);
std::complex<double> z2 = a - z;
printf("%.17g + %.17g i\n", z2.real(), z2.imag());
std::complex<double> z3 = z - a;
printf("%.17g + %.17g i\n", z3.real(), z3.imag());
}
Both yield:
4 + -3 i
-4 + 3 i
Note that Complex::add(double) exists. This needs no alternative as the
addition of the real to the real component is commutative:
double a
complex z
a + z = z + a
The subtraction is not commutative (as shown above):
a - z != z - a
So given the Complex has a subtract(double) it would be fair to have a
subtractFrom(double).
The complex subtraction is also not commutative:
complex y
complex z
y - z != z - y
But in this case you can achieve the same with the API by swapping the
arguments.
>
>>
>> This would avoid:
>>
>> Complex a = …;
>> double b = …;
>> Complex c = Complex.ofCartesian(b - a.real(), a.imag());
>
> This is clear.
>
>> Complex d = Complex.ofReal(b).subtract(a).conj();
>
> This even more, but, I get it, with 3 instantiations instead of 1.
>
>>
>> With
>>
>> Complex a = …;
>> double b = …;
>> Complex c = a.subtractFrom(b);
>
> Looks ambiguous... But if it's standard naming.
>
> Regards,
> Gilles
>
>>
>>>
>>>>
>>>>
>>>> [1] http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf
>>>> <http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf>
>>>>
>>>> [2] http://www.open-std.org/JTC1/SC22/WG14/www/standards
>>>> <http://www.open-std.org/JTC1/SC22/WG14/www/standards>
>>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> <mailto:dev-unsubscr...@commons.apache.org>
> For additional commands, e-mail: dev-h...@commons.apache.org
> <mailto:dev-h...@commons.apache.org>
