search

evristzam / ndt

Automatically exported from code.google.com/p/ndt
Other
0 stars 0 forks source link

Same code for IF and ELSE branches in code printing test results. #98

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
https://code.google.com/p/ndt/source/browse/trunk/Applet/Tcpbw100.java#3266

Both IF and ELSE branches have the same code.
AFAICT from other parts of this file with similar code (for example 
https://code.google.com/p/ndt/source/browse/trunk/Applet/Tcpbw100.java#3022), 
the desired code is probably supposed to only have the IF branch. I.e., 

if (_dC2sspd > _dSc2sspd) {
  if (_dSc2sspd < (_dC2sspd * (1.0 - NDTConstants.VIEW_DIFF))) {
    _txtStatistics.append(_resBundDisplayMsgs
      .getString("c2s")
      + " "
      + _resBundDisplayMsgs.getString("qSeen")
      + ": "
      + NDTUtils.prtdbl(NDTConstants.PERCENTAGE * (_dC2sspd - _dSc2sspd)
                        / _dC2sspd) + "%\n");
  }
}

Same problem at line:
https://code.google.com/p/ndt/source/browse/trunk/Applet/Tcpbw100.java#3291

Original issue reported on code.google.com by tizi...@google.com on 21 Nov 2013 at 12:23

GoogleCodeExporter commented 9 years ago 
This code fragment looked like this some time ago, so there was a small 
difference between IF and ELSE branches:

if ((tests & TEST_C2S) == TEST_C2S) {
        if (c2sspd > sc2sspd) {
          if (sc2sspd < (c2sspd  * (1.0 - VIEW_DIFF))) {
            statistics.append("C2S throughput test: Excessive packet queuing detected: " + prtdbl(100 * (c2sspd - sc2sspd) / c2sspd) + "%\n");
          }
          else {
            statistics.append("C2S throughput test: Packet queuing detected: " + prtdbl(100 * (c2sspd - sc2sspd) / c2sspd) + "%\n");
          }
        }
      }

Original comment by jslawin...@soldevelo.com on 21 Nov 2013 at 12:37

GoogleCodeExporter commented 9 years ago 
Thanks for the quick answer!
What's the preferred way to fix this?
1) Re-introduce the distinction,
2) Eliminate the ELSE branch, OR
3) Eliminate the check -- IMO, this is the least likely, by looking at the rest 
of the code.

Original comment by tizi...@google.com on 21 Nov 2013 at 3:08

GoogleCodeExporter commented 9 years ago 
I think the first way is the best. The idea around this distinction was to 
highlight the excessive packet queueing (i.e. more 10% of the packets were 
queued) and I do not see any reason why we shouldn't highlight this fact to the 
end user.

Original comment by jslawin...@soldevelo.com on 21 Nov 2013 at 3:35

GoogleCodeExporter commented 9 years ago 
I concur that it's better to highlight the excessiveness.

Original comment by AaronMat...@gmail.com on 26 Nov 2013 at 1:08

GoogleCodeExporter commented 9 years ago

Original comment by jslawin...@soldevelo.com on 28 Nov 2013 at 8:58

GoogleCodeExporter commented 9 years ago

Original comment by smale...@soldevelo.com on 17 Feb 2014 at 10:37

GoogleCodeExporter commented 9 years ago 
Changes available on branch applet_98. Feel free to review.

Original comment by smale...@soldevelo.com on 17 Feb 2014 at 1:03

GoogleCodeExporter commented 9 years ago

Original comment by smale...@soldevelo.com on 4 Mar 2014 at 8:45

GoogleCodeExporter commented 9 years ago

Original comment by skost...@soldevelo.com on 23 Jun 2014 at 5:49