Fix container inefficiency in FlowArrangement.java #415
Conversation
| y = y + size.height + this.verticalGap; | ||
| } | ||
| else { | ||
| // start new row |
There was a problem hiding this comment.
Should we not reinitialize haveItemsInRow to false here?
There was a problem hiding this comment.
The original code clears the itemsInRow and adds an element to the itemsInRow afterwards in the same code block.
So I think the reinitialization is not required.
| * @return A list of datasets. | ||
| */ | ||
| private List<XYDataset<S>> getDatasetsMappedToDomainAxis(Integer axisIndex) { | ||
| private List<XYDataset<S>> getDatasetsMappedToDomainAxis(Integer axisIndex, List<XYDataset<S>> result) { |
There was a problem hiding this comment.
Not sure about efficiency, but IMHO, this makes the contract of these methods worse. Previously these were pure functions without side effects, but now there are side effects introduced. I would be pretty careful here if any performance improvement warrants changing the contract of the methods like this.
There was a problem hiding this comment.
Got it!
If such changes cause some concerns, I will undo these changes in the next commit.
cinsttool
changed the title
cinsttool
changed the title
|
@RockyMM Thank you for your comments! We have reverted the XYPlot.java and updated the pr title&description. Could you please approve this pr? |
2 participants
Hi,
We find that there exist container inefficiency in FlowArrangement.java.
In FlowArrangement.java, the List object
itemsInRowonly performs add, clear, and isEmpty operations.And the objects in this list are never accessed.
Therefore, we can replace this list with a boolean variable.
We discovered the above container inefficiency using our tool cinst. The patch is submitted.
Could you please check and accept it? We have tested the patch on our PC. The patched program works well.