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