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