RFR: JDK-8293364: IGV: Refactor Action in EditorTopComponent and fix minor bugs [v8]
Tobias Holenstein
tholenstein at openjdk.org
Tue Sep 20 13:06:30 UTC 2022
On Mon, 19 Sep 2022 10:12:27 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:
>> Tobias Holenstein has updated the pull request incrementally with one additional commit since the last revision:
>>
>> undo removing variable generated by form editor
>
> I only have some code style specific comments. Otherwise, good cleanup!
@chhagedorn I integrated your suggestions now
> src/utils/IdealGraphVisualizer/View/src/main/java/com/sun/hotspot/igv/view/EditorTopComponent.java line 379:
>
>> 377:
>> 378: @Override
>> 379: public void componentOpened() { }
>
> Is this empty override required? Otherwise, the method can be removed.
I removed it
> src/utils/IdealGraphVisualizer/View/src/main/java/com/sun/hotspot/igv/view/actions/ExpandPredecessorsAction.java line 44:
>
>> 42: EditorTopComponent editor = EditorTopComponent.getActive();
>> 43: if (editor != null) {
>> 44: Set<Figure> oldSelection = editor.getModel().getSelectedFigures();
>
> This code looks very similar to the code in `ExpandSuccessorAction`. The only difference is `f.getSuccessors()` vs `f.getPredecessors()`. Moreover, the only other difference between this class and `ExpandSuccessorAction` seems to be `getName()`. I suggest to create a super class `ExpandAdjacentAction` (or another name that fits) for these two classes and have a common `expandAdjacent()` method (or another name that fits) for this code that just takes the predecessors or successors as parameter.
I agree. I created a new `ExpandAdjacentAction` superclass now
-------------
PR: https://git.openjdk.org/jdk/pull/10170
More information about the hotspot-compiler-dev
mailing list