-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Caching the length/size in a variable for collections in order to avo… #6453
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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--) { | ||
int rowSelected = rowsSelected[i]; | ||
tableModel.moveRow(rowSelected, rowSelected + 1, rowSelected + 1); | ||
} | ||
|
@@ -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--) { | ||
Comment on lines
+538
to
+540
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suggest we revert the change as well as it hardly yields any performance gains, yet it makes the code harder to follow |
||
tableModel.removeRow(rowsSelected[i]); | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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--) { | ||
Comment on lines
+101
to
+102
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please revert as discussed previously. |
||
guiPackage.getTreeModel().getCurrentSubTree(nodes[i]).traverse(this); | ||
} | ||
} finally { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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--) { | ||
Comment on lines
+59
to
+60
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
JMeterTreeNode copiedNode = copiedNodes[nodeIndex]; | ||
int index = parentNode.getIndex(currentNode) + 1; | ||
treeModel.insertNodeInto(copiedNode, parentNode, index); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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--) { | ||
Comment on lines
+81
to
+82
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please revert |
||
removeNode(nodes[i]); | ||
} | ||
guiPackage.getTreeListener().getJTree().setSelectionPath(newTreePath); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Frankly, I think the performance gains are negligible here, and the change makes the code slightly harder to follow
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hi @vlsi I think the change improves readability and maintainability. and It does not affect functionality. Are you still agree with reverting changes?