Re: svn commit: r1507449 - /jmeter/trunk/src/components/org/apache/jmeter/visualizers/RespTimeGraphChart.java

[sebb <se...@gmail.com>]

On 26 July 2013 22:17,  <pm...@apache.org> wrote:
> Author: pmouawad
> Date: Fri Jul 26 21:17:16 2013
> New Revision: 1507449
>
> URL: http://svn.apache.org/r1507449
> Log:
> Fixed:
> - String appending
> - Wrong division
> - Primitive value is boxed then unboxed to perform primitive coercion
>
> Modified:
>     jmeter/trunk/src/components/org/apache/jmeter/visualizers/RespTimeGraphChart.java
>
> Modified: jmeter/trunk/src/components/org/apache/jmeter/visualizers/RespTimeGraphChart.java
> URL: http://svn.apache.org/viewvc/jmeter/trunk/src/components/org/apache/jmeter/visualizers/RespTimeGraphChart.java?rev=1507449&r1=1507448&r2=1507449&view=diff
> ==============================================================================
> --- jmeter/trunk/src/components/org/apache/jmeter/visualizers/RespTimeGraphChart.java (original)
> +++ jmeter/trunk/src/components/org/apache/jmeter/visualizers/RespTimeGraphChart.java Fri Jul 26 21:17:16 2013
> @@ -322,7 +322,7 @@ public class RespTimeGraphChart extends
>
>              // Y Axis ruler
>              try {
> -                double numInterval = _height / 50; // ~a tic every 50 px
> +                double numInterval = _height / 50d; // ~a tic every 50 px
>                  double incrYAxis = max / numInterval;
>                  double incrTopValue = _incrScaleYAxis;
>                  if (_incrScaleYAxis == 0) {
> @@ -332,7 +332,7 @@ public class RespTimeGraphChart extends
>                      incrTopValue = 1.0d; // Increment cannot be < 1
>                  }
>                  yaxis.setUserDefinedScale(0, incrTopValue);
> -                yaxis.setNumItems(new Double(max / incrTopValue).intValue() + 1);
> +                yaxis.setNumItems((int)(max / incrTopValue) + 1);
>                  yaxis.setShowGridLines(1);
>              } catch (PropertyException e) {
>                  log.warn("",e);
> @@ -367,11 +367,12 @@ public class RespTimeGraphChart extends
>
>      private int getTopValue(double value, int roundMode) {
>          String maxStr = String.valueOf(Math.round(value));
> -        String divValueStr = "1"; //$NON-NLS-1$
> +        StringBuilder divValueStr = new StringBuilder(maxStr.length()+1);
> +        divValueStr.append("1");
>          for (int i = 1; i < maxStr.length(); i++) {
> -            divValueStr += "0"; //$NON-NLS-1$
> +            divValueStr.append("0"); //$NON-NLS-1$
>          }
> -        int divValueInt = Integer.parseInt(divValueStr);
> +        int divValueInt = Integer.parseInt(divValueStr.toString());

I agree it is better to use StringBuilder rather than concatenation,
but the original code looks very odd.

It looks like it is calculating a power of 10 using a string! Can that be right?
If so, it would be better to drop the conversion to/from the string entirely.

The method needs a bit of documentation to explain what it is trying to do.

>          BigDecimal round = new BigDecimal(value / divValueInt);
>          round = round.setScale(0, roundMode);
>          int topValue = round.intValue() * divValueInt;
>
>