You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Gilles Sadowski <gi...@harfang.homelinux.org> on 2012/08/05 15:28:37 UTC
Re: svn commit: r1369540 -
/commons/proper/math/trunk/src/main/java/org/apache/commons/math3/optimization/linear/SimplexSolver.java
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