On Sun, Aug 05, 2012 at 09:32:46AM -0000, t...@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-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org

Reply via email to