commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gilles Sadowski <gil...@harfang.homelinux.org>
Subject Re: svn commit: r1369540 - /commons/proper/math/trunk/src/main/java/org/apache/commons/math3/optimization/linear/SimplexSolver.java
Date Sun, 05 Aug 2012 13:28:37 GMT
On Sun, Aug 05, 2012 at 09:32:46AM -0000, tn@apache.org wrote:
> Author: tn
> Date: Sun Aug  5 09:32:46 2012
> New Revision: 1369540
> 
> URL: http://svn.apache.org/viewvc?rev=1369540&view=rev
> Log:
> Fixed findbugs finding when comparing Integer references.
> 
> Modified:
>     commons/proper/math/trunk/src/main/java/org/apache/commons/math3/optimization/linear/SimplexSolver.java
> 
> Modified: commons/proper/math/trunk/src/main/java/org/apache/commons/math3/optimization/linear/SimplexSolver.java
> URL: http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math3/optimization/linear/SimplexSolver.java?rev=1369540&r1=1369539&r2=1369540&view=diff
> ==============================================================================
> --- commons/proper/math/trunk/src/main/java/org/apache/commons/math3/optimization/linear/SimplexSolver.java
(original)
> +++ commons/proper/math/trunk/src/main/java/org/apache/commons/math3/optimization/linear/SimplexSolver.java
Sun Aug  5 09:32:46 2012
> @@ -142,8 +142,9 @@ public class SimplexSolver extends Abstr
>                  int minIndex = tableau.getWidth();
>                  for (Integer row : minRatioPositions) {
>                      int i = tableau.getNumObjectiveFunctions();
> -                    for (; i < tableau.getWidth() - 1 && minRow != row; i++)
{
> -                        if (row == tableau.getBasicRow(i)) {
> +                    for (; i < tableau.getWidth() - 1 && !row.equals(minRow);
i++) {
                         ^^^^^^^^
This is a needlessly not standard way to write a for-loop. [Better to
tighten the scope the loop variable.]
> +                        Integer basicRow = tableau.getBasicRow(i);
> +                        if (basicRow != null && basicRow.equals(row)) {
>                              if (i < minIndex) {
>                                  minIndex = i;
>                                  minRow = row;
> 
> 

Any objection to rewrite as

                    for (int i = tableau.getNumObjectiveFunctions();
                         i < tableau.getWidth() - 1 &&
                         !row.equals(minRow)
                         i++) {

?

If I correctly read the code of that functions, it would be slightly more
efficient to call once "tableau.getNumObjectiveFunctions()" at the top of
the method and reuse a local variable:

final int numObjFunc = tableau.getNumObjectiveFunctions();

// ...

                    for (int i = numObjFunc;
                         i < tableau.getWidth() - 1 &&
                         !row.equals(minRow)
                         i++) {

And similarly for "tableau.getWidth() - 1".
[IMHO, little changes like those make the code more readable and,
consequently, help to locate bugs in the long run.]


Thanks,
Gilles

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message