java 8 improvements. by arturobernalg · Pull Request #217 · apache/commons-math

arturobernalg · 2022-10-20T20:53:04Z

Replaced Anonymous type with lambda
Replace Collections.sort() with List.sort()
Replace Lambda with method reference
Simplifiable 'Map' operations

codecov-commenter · 2022-10-20T20:57:12Z

Codecov Report

Merging #217 (5fbb985) into master (ec7eb8b) will decrease coverage by 0.01%.
The diff coverage is 93.54%.

❗ Current head 5fbb985 differs from pull request most recent head 05c89f4. Consider uploading reports for the commit 05c89f4 to get more accurate results

@@             Coverage Diff              @@
##             master     #217      +/-   ##
============================================
- Coverage     86.55%   86.53%   -0.02%     
+ Complexity     9810     9798      -12     
============================================
  Files           533      533              
  Lines         35579    35559      -20     
  Branches       6212     6207       -5     
============================================
- Hits          30796    30772      -24     
+ Misses         3524     3523       -1     
- Partials       1259     1264       +5

Impacted Files	Coverage Δ
...s/math4/legacy/ml/clustering/ClusterEvaluator.java	`50.00% <ø> (+16.66%)`	⬆️
...e/commons/math4/transform/FastCosineTransform.java	`100.00% <ø> (ø)`
...che/commons/math4/transform/FastSineTransform.java	`100.00% <ø> (ø)`
...ommons/math4/legacy/fitting/SimpleCurveFitter.java	`74.07% <50.00%> (ø)`
...y/ode/nonstiff/AdamsNordsieckFieldTransformer.java	`98.61% <66.66%> (-1.39%)`	⬇️
...egacy/analysis/differentiation/SparseGradient.java	`96.88% <100.00%> (+0.57%)`	⬆️
...ons/math4/legacy/analysis/solvers/BrentSolver.java	`77.27% <100.00%> (ø)`
...pache/commons/math4/legacy/genetics/RandomKey.java	`89.55% <100.00%> (ø)`
...linear/scalar/MultiStartMultivariateOptimizer.java	`73.68% <100.00%> (ø)`
...linear/scalar/noderiv/HedarFukushimaTransform.java	`69.76% <100.00%> (ø)`
... and 8 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

aherbert

Thanks for the updates. A few small changes required.

Note that CI is failing on checkstyle unused imports.

aherbert · 2022-10-20T21:04:42Z

...c/main/java/org/apache/commons/math4/legacy/ode/nonstiff/AdamsNordsieckFieldTransformer.java

-            shiftedP[i] = shiftedP[i - 1];
-        }
+        // shift rows
+        if (shiftedP.length - 1 >= 0) System.arraycopy(shiftedP, 0, shiftedP, 1, shiftedP.length - 1);


Should this be if (shiftedP.length -1 > 0) (since we cannot copy length=0).

Please add enclosing braces.

aherbert · 2022-10-20T21:08:02Z

...ons-math-transform/src/main/java/org/apache/commons/math4/transform/FastCosineTransform.java

            return normalization == Norm.ORTHO ?
                f -> TransformUtils.scaleInPlace(fct(f), Math.sqrt(2d / (f.length - 1))) :
-                f -> fct(f);
+                    this::fct;


Whitespace indentation

aherbert · 2022-10-20T21:08:12Z

commons-math-transform/src/main/java/org/apache/commons/math4/transform/FastSineTransform.java

            return normalization == Norm.ORTHO ?
                f -> TransformUtils.scaleInPlace(fst(f), Math.sqrt(2d / f.length)) :
-                f -> fst(f);
+                    this::fst;


Indentation again

arturobernalg · 2022-10-20T21:18:02Z

done. @aherbert

aherbert · 2022-10-20T21:25:25Z

Still failing on the same checkstyle violation. Run locally: mvn checkstyle:check and fix the neuralnetworks module (and maybe others).

arturobernalg · 2022-10-21T16:33:43Z

Still failing on the same checkstyle violation. Run locally: mvn checkstyle:check and fix the neuralnetworks module (and maybe others).

hi @aherbert
Done

aherbert

Some minor reversions please. Thanks.

aherbert · 2022-10-21T17:07:23Z

...y/src/main/java/org/apache/commons/math4/legacy/analysis/differentiation/SparseGradient.java

    public SparseGradient copySign(final SparseGradient sign) {
        final long m = Double.doubleToLongBits(value);
        final long s = Double.doubleToLongBits(sign.value);
-        if ((m >= 0 && s >= 0) || (m < 0 && s < 0)) { // Sign is currently OK


Didn't notice this before. I would not remove the extra parentheses as it makes it obvious over knowing the operator precedence of && over ||.

aherbert · 2022-10-21T17:07:33Z

...y/src/main/java/org/apache/commons/math4/legacy/analysis/differentiation/SparseGradient.java

    public SparseGradient copySign(final double sign) {
        final long m = Double.doubleToLongBits(value);
        final long s = Double.doubleToLongBits(sign);
-        if ((m >= 0 && s >= 0) || (m < 0 && s < 0)) { // Sign is currently OK


Again please revert

aherbert · 2022-10-21T17:08:08Z

...ons-math-legacy/src/main/java/org/apache/commons/math4/legacy/fitting/SimpleCurveFitter.java

        private boolean isBetween(double value,
                                  double boundary1,
                                  double boundary2) {
-            return (value >= boundary1 && value <= boundary2) ||


Please revert

aherbert · 2022-10-21T17:08:42Z

...c/main/java/org/apache/commons/math4/legacy/ode/nonstiff/AdamsNordsieckFieldTransformer.java

-            // shift rows
-            shiftedP[i] = shiftedP[i - 1];
+        // shift rows
+        if (shiftedP.length - 1 >= 0) {


aherbert

A couple of whitespace issues to fix please

aherbert · 2022-10-22T07:08:33Z

...ons-math-legacy/src/main/java/org/apache/commons/math4/legacy/fitting/SimpleCurveFitter.java

                                  double boundary2) {
            return (value >= boundary1 && value <= boundary2) ||
-                (value >= boundary2 && value <= boundary1);
+                    (value >= boundary2 && value <= boundary1);


Please revert the indentation too

aherbert · 2022-10-22T07:08:48Z

...ath-legacy/src/main/java/org/apache/commons/math4/legacy/ml/clustering/ClusterEvaluator.java

        return eval.isBetterScore(1, 2) ?
            clusters -> 1 / eval.score(clusters) :
-            clusters -> eval.score(clusters);
+                eval::score;


This has incorrect indentation

* Replace Collections.sort() with List.sort() * Replace Lambda with method reference * Simplifiable 'Map' operations

aherbert requested changes Oct 20, 2022

View reviewed changes

arturobernalg requested a review from aherbert October 20, 2022 21:17

aherbert requested changes Oct 21, 2022

View reviewed changes

arturobernalg requested a review from aherbert October 21, 2022 19:34

aherbert requested changes Oct 22, 2022

View reviewed changes

* Replaced Anonymous type with lambda

c917d64

* Replace Collections.sort() with List.sort() * Replace Lambda with method reference * Simplifiable 'Map' operations

arturobernalg requested a review from aherbert October 22, 2022 19:04

arturobernalg closed this by deleting the head repository Jun 21, 2023

Conversation

arturobernalg commented Oct 20, 2022

Uh oh!

codecov-commenter commented Oct 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

aherbert left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

arturobernalg commented Oct 20, 2022

Uh oh!

aherbert commented Oct 20, 2022

Uh oh!

arturobernalg commented Oct 21, 2022

Uh oh!

aherbert left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aherbert left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov-commenter commented Oct 20, 2022 •

edited

Loading