vlsi commented on code in PR #6453:
URL: https://github.com/apache/jmeter/pull/6453#discussion_r2217366196


##########
src/core/src/main/java/org/apache/jmeter/config/gui/ArgumentsPanel.java:
##########
@@ -441,10 +441,10 @@ private void moveDown() {
         // or the selected rows will be unselected
         int[] rowsSelected = table.getSelectedRows();
         GuiUtils.stopTableEditing(table);
-
-        if (rowsSelected.length > 0 && rowsSelected[rowsSelected.length - 1] < 
table.getRowCount() - 1) {
+        int selectedRowsCount = rowsSelected.length;
+        if (selectedRowsCount > 0 && rowsSelected[selectedRowsCount - 1] < 
table.getRowCount() - 1) {
             table.clearSelection();
-            for (int i = rowsSelected.length - 1; i >= 0; i--) {
+            for (int i = selectedRowsCount - 1; i >= 0; i--) {

Review Comment:
   Frankly, I think the performance gains are negligible here, and the change 
makes the code slightly harder to follow



##########
src/core/src/main/java/org/apache/jmeter/gui/action/Duplicate.java:
##########
@@ -56,7 +56,8 @@ public void doAction(ActionEvent e) {
         JMeterTreeNode currentNode = treeListener.getCurrentNode();
         JMeterTreeNode parentNode = (JMeterTreeNode) currentNode.getParent();
         JMeterTreeModel treeModel = instance.getTreeModel();
-        for (int nodeIndex = copiedNodes.length - 1; nodeIndex >= 0; 
nodeIndex--) {
+        int copiedNodesCount = copiedNodes.length;
+        for (int nodeIndex = copiedNodesCount - 1; nodeIndex >= 0; 
nodeIndex--) {

Review Comment:
   `copiedNodes.length` was computed one-time only, there's no point in adding 
a variable



##########
src/core/src/main/java/org/apache/jmeter/config/gui/ArgumentsPanel.java:
##########
@@ -535,8 +535,9 @@ protected void deleteArgument() {
         int[] rowsSelected = table.getSelectedRows();
         int anchorSelection = 
table.getSelectionModel().getAnchorSelectionIndex();
         table.clearSelection();
-        if (rowsSelected.length > 0) {
-            for (int i = rowsSelected.length - 1; i >= 0; i--) {
+        int selectedRowsCount = rowsSelected.length;
+        if (selectedRowsCount > 0) {
+            for (int i = selectedRowsCount - 1; i >= 0; i--) {

Review Comment:
   I suggest we revert the change as well as it hardly yields any performance 
gains, yet it makes the code harder to follow



##########
src/core/src/main/java/org/apache/jmeter/gui/action/CheckDirty.java:
##########
@@ -98,7 +98,8 @@ public void doAction(ActionEvent e) {
             JMeterTreeNode[] nodes = 
guiPackage.getTreeListener().getSelectedNodes();
             removeMode = true;
             try {
-                for (int i = nodes.length - 1; i >= 0; i--) {
+                int selectedNodesCount = nodes.length;
+                for (int i = selectedNodesCount - 1; i >= 0; i--) {

Review Comment:
   Please revert as discussed previously.



##########
src/core/src/main/java/org/apache/jmeter/gui/action/Remove.java:
##########
@@ -78,7 +78,8 @@ public void doAction(ActionEvent e) {
             JMeterTreeNode[] nodes = 
guiPackage.getTreeListener().getSelectedNodes();
             TreePath newTreePath = // Save parent node for later
             guiPackage.getTreeListener().removedSelectedNode();
-            for (int i = nodes.length - 1; i >= 0; i--) {
+            int nodesCount = nodes.length;
+            for (int i = nodesCount - 1; i >= 0; i--) {

Review Comment:
   Please revert



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@jmeter.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to